Merge pull request #890 from ethcore/fix_875

fixed #875 and added tests for eth_sendTransaction

miner/src/lib.rs

@@ -108,6 +108,9 @@ pub trait MinerService : Send + Sync {
 	/// Query pending transactions for hash
 	fn transaction(&self, hash: &H256) -> Option<SignedTransaction>;
 
+	/// Returns highest transaction nonce for given address.
+	fn last_nonce(&self, address: &Address) -> Option<U256>;
+
 	/// Suggested gas price
 	fn sensible_gas_price(&self) -> U256 { x!(20000000000u64) }
 }

miner/src/miner.rs

@@ -228,6 +228,10 @@ impl MinerService for Miner {
 		queue.find(hash)
 	}
 
+	fn last_nonce(&self, address: &Address) -> Option<U256> {
+		self.transaction_queue.lock().unwrap().last_nonce(address)
+	}
+
 	fn update_sealing(&self, chain: &BlockChainClient) {
 		if self.sealing_enabled.load(atomic::Ordering::Relaxed) {
 			let current_no = chain.chain_info().best_block_number;

miner/src/transaction_queue.rs

@@ -523,6 +523,11 @@ impl TransactionQueue {
 		self.last_nonces.clear();
 	}
 
+	/// Returns highest transaction nonce for given address.
+	pub fn last_nonce(&self, address: &Address) -> Option<U256> {
+		self.last_nonces.get(address).cloned()
+	}
+
 	/// Checks if there are any transactions in `future` that should actually be promoted to `current`
 	/// (because nonce matches).
 	fn move_matching_future_to_current(&mut self, address: Address, mut current_nonce: U256, first_nonce: U256) {
@@ -1255,4 +1260,29 @@ mod test {
 		assert_eq!(stats.future, 0);
 		assert_eq!(stats.pending, 1);
 	}
+
+	#[test]
+	fn should_return_none_when_transaction_from_given_address_does_not_exist() {
+		// given
+		let mut txq = TransactionQueue::new();
+
+		// then
+		assert_eq!(txq.last_nonce(&Address::default()), None);
+	}
+
+	#[test]
+	fn should_return_correct_nonce_when_transactions_from_given_address_exist() {
+		// given
+		let mut txq = TransactionQueue::new();
+		let tx = new_tx();
+		let from = tx.sender().unwrap();
+		let nonce = tx.nonce;
+		let details = |a: &Address| AccountDetails { nonce: nonce, balance: !U256::zero() };
+
+		// when
+		txq.add(tx, &details).unwrap();
+
+		// then
+		assert_eq!(txq.last_nonce(&from), Some(nonce));
+	}
 }

rpc/src/v1/impls/eth.rs

@@ -188,11 +188,15 @@ impl<C, S, A, M, EM> EthClient<C, S, A, M, EM>
 
 	fn dispatch_transaction(&self, signed_transaction: SignedTransaction, raw_transaction: Vec<u8>) -> Result<Value, Error> {
 		let hash = signed_transaction.hash();
-		
+
 		let import = {
+			let miner = take_weak!(self.miner);
 			let client = take_weak!(self.client);
 			take_weak!(self.miner).import_transactions(vec![signed_transaction], |a: &Address| AccountDetails {
-				nonce: client.nonce(a),
+				nonce: miner
+					.last_nonce(a)
+					.map(|nonce| nonce + U256::one())
+					.unwrap_or_else(|| client.nonce(a)),
 				balance: client.balance(a),
 			})
 		};
@@ -488,7 +492,11 @@ impl<C, S, A, M, EM> Eth for EthClient<C, S, A, M, EM>
 							let client = take_weak!(self.client);
 							let miner = take_weak!(self.miner);
 							EthTransaction {
-								nonce: request.nonce.unwrap_or_else(|| client.nonce(&request.from)),
+								nonce: request.nonce
+									.or_else(|| miner
+											 .last_nonce(&request.from)
+											 .map(|nonce| nonce + U256::one()))
+									.unwrap_or_else(|| client.nonce(&request.from)),
 								action: request.to.map_or(Action::Create, Action::Call),
 								gas: request.gas.unwrap_or_else(default_gas),
 								gas_price: request.gas_price.unwrap_or_else(|| miner.sensible_gas_price()),

rpc/src/v1/impls/personal.rs

@@ -49,7 +49,7 @@ impl<A> Personal for PersonalClient<A> where A: AccountProvider + 'static {
 			|(pass, )| {
 				let store = take_weak!(self.accounts);
 				match store.new_account(&pass) {
-					Ok(address) => Ok(Value::String(format!("0x{:?}", address))),
+					Ok(address) => to_value(&address),
 					Err(_) => Err(Error::internal_error())
 				}
 			}

rpc/src/v1/tests/eth.rs

@@ -23,6 +23,7 @@ use util::numbers::{Uint, U256};
 use ethcore::client::{TestBlockChainClient, EachBlockWith, Executed, TransactionId};
 use ethcore::log_entry::{LocalizedLogEntry, LogEntry};
 use ethcore::receipt::LocalizedReceipt;
+use ethcore::transaction::{Transaction, Action};
 use v1::{Eth, EthClient};
 use v1::tests::helpers::{TestAccount, TestAccountProvider, TestSyncProvider, Config, TestMinerService, TestExternalMiner};
 
@@ -52,7 +53,7 @@ fn miner_service() -> Arc<TestMinerService> {
 struct EthTester {
 	pub client: Arc<TestBlockChainClient>,
 	pub sync: Arc<TestSyncProvider>,
-	_accounts_provider: Arc<TestAccountProvider>,
+	pub accounts_provider: Arc<TestAccountProvider>,
 	miner: Arc<TestMinerService>,
 	hashrates: Arc<RwLock<HashMap<H256, U256>>>,
 	pub io: IoHandler,
@@ -72,7 +73,7 @@ impl Default for EthTester {
 		EthTester {
 			client: client,
 			sync: sync,
-			_accounts_provider: ap,
+			accounts_provider: ap,
 			miner: miner,
 			io: io,
 			hashrates: hashrates,
@@ -453,9 +454,53 @@ fn rpc_eth_estimate_gas_default_block() {
 }
 
 #[test]
-#[ignore]
 fn rpc_eth_send_transaction() {
-	unimplemented!()
+	let account = TestAccount::new("123");
+	let address = account.address();
+	let secret = account.secret.clone();
+
+	let tester = EthTester::default();
+	tester.accounts_provider.accounts.write().unwrap().insert(address.clone(), account);
+	let request = r#"{
+		"jsonrpc": "2.0",
+		"method": "eth_sendTransaction",
+		"params": [{
+			"from": ""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"",
+			"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
+			"gas": "0x76c0",
+			"gasPrice": "0x9184e72a000",
+			"value": "0x9184e72a"
+		}],
+		"id": 1
+	}"#;
+
+	let t = Transaction {
+		nonce: U256::zero(),
+		gas_price: U256::from(0x9184e72a000u64),
+		gas: U256::from(0x76c0),
+		action: Action::Call(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()),
+		value: U256::from(0x9184e72au64),
+		data: vec![]
+	}.sign(&secret);
+
+	let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", t.hash()).as_ref() + r#"","id":1}"#;
+
+	assert_eq!(tester.io.handle_request(request.as_ref()), Some(response));
+
+	tester.miner.last_nonces.write().unwrap().insert(address.clone(), U256::zero());
+
+	let t = Transaction {
+		nonce: U256::one(),
+		gas_price: U256::from(0x9184e72a000u64),
+		gas: U256::from(0x76c0),
+		action: Action::Call(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()),
+		value: U256::from(0x9184e72au64),
+		data: vec![]
+	}.sign(&secret);
+
+	let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", t.hash()).as_ref() + r#"","id":1}"#;
+
+	assert_eq!(tester.io.handle_request(request.as_ref()), Some(response));
 }
 
 #[test]

rpc/src/v1/tests/helpers/account_provider.rs

@@ -20,7 +20,7 @@ use std::sync::RwLock;
 use std::collections::HashMap;
 use std::io;
 use util::hash::{Address, H256, FixedHash};
-use util::crypto::{Secret, Signature};
+use util::crypto::{Secret, Signature, KeyPair};
 use util::keys::store::{AccountProvider, SigningError, EncryptedHashMapError};
 
 /// Account mock.
@@ -30,23 +30,31 @@ pub struct TestAccount {
 	pub unlocked: bool,
 	/// Account's password.
 	pub password: String,
+	/// Account's secret.
+	pub secret: Secret,
 }
 
 impl TestAccount {
 	/// Creates new test account.
 	pub fn new(password: &str) -> Self {
+		let pair = KeyPair::create().unwrap();
 		TestAccount {
 			unlocked: false,
 			password: password.to_owned(),
+			secret: pair.secret().clone()
 		}
 	}
+
+	/// Returns account address.
+	pub fn address(&self) -> Address {
+		KeyPair::from_secret(self.secret.clone()).unwrap().address()
+	}
 }
 
 /// Test account provider.
 pub struct TestAccountProvider {
-	accounts: RwLock<HashMap<Address, TestAccount>>,
-	/// Added accounts passwords.
-	pub adds: RwLock<Vec<String>>,
+	/// Test provider accounts.
+	pub accounts: RwLock<HashMap<Address, TestAccount>>,
 }
 
 impl TestAccountProvider {
@@ -54,7 +62,6 @@ impl TestAccountProvider {
 	pub fn new(accounts: HashMap<Address, TestAccount>) -> Self {
 		TestAccountProvider {
 			accounts: RwLock::new(accounts),
-			adds: RwLock::new(vec![]),
 		}
 	}
 }
@@ -76,14 +83,20 @@ impl AccountProvider for TestAccountProvider {
 	}
 
 	fn new_account(&self, pass: &str) -> Result<Address, io::Error> {
-		let mut adds = self.adds.write().unwrap();
-		let address = Address::from(adds.len() as u64 + 2);
-		adds.push(pass.to_owned());
+		let account = TestAccount::new(pass);
+		let address = KeyPair::from_secret(account.secret.clone()).unwrap().address();
+		self.accounts.write().unwrap().insert(address.clone(), account);
 		Ok(address)
 	}
 
-	fn account_secret(&self, _account: &Address) -> Result<Secret, SigningError> {
-		Ok(Secret::random())
+	fn account_secret(&self, address: &Address) -> Result<Secret, SigningError> {
+		// todo: consider checking if account is unlock. some test may need alteration then.
+		self.accounts
+			.read()
+			.unwrap()
+			.get(address)
+			.ok_or(SigningError::NoAccount)
+			.map(|acc| acc.secret.clone())
 	}
 
 	fn sign(&self, _account: &Address, _message: &H256) -> Result<Signature, SigningError> {

rpc/src/v1/tests/helpers/miner_service.rs

@@ -16,7 +16,7 @@
 
 //! Test implementation of miner service.
 
-use util::{Address, H256, Bytes};
+use util::{Address, H256, Bytes, U256};
 use util::standard::*;
 use ethcore::error::Error;
 use ethcore::client::BlockChainClient;
@@ -27,19 +27,22 @@ use ethminer::{MinerService, MinerStatus, AccountDetails};
 /// Test miner service.
 pub struct TestMinerService {
 	/// Imported transactions.
-	pub imported_transactions: RwLock<Vec<H256>>,
+	pub imported_transactions: Mutex<Vec<SignedTransaction>>,
 	/// Latest closed block.
 	pub latest_closed_block: Mutex<Option<ClosedBlock>>,
 	/// Pre-existed pending transactions
 	pub pending_transactions: Mutex<HashMap<H256, SignedTransaction>>,
+	/// Last nonces.
+	pub last_nonces: RwLock<HashMap<Address, U256>>,
 }
 
 impl Default for TestMinerService {
 	fn default() -> TestMinerService {
 		TestMinerService {
-			imported_transactions: RwLock::new(Vec::new()),
+			imported_transactions: Mutex::new(Vec::new()),
 			latest_closed_block: Mutex::new(None),
 			pending_transactions: Mutex::new(HashMap::new()),
+			last_nonces: RwLock::new(HashMap::new()),
 		}
 	}
 }
@@ -56,28 +59,52 @@ impl MinerService for TestMinerService {
 	}
 
 	/// Imports transactions to transaction queue.
-	fn import_transactions<T>(&self, _transactions: Vec<SignedTransaction>, _fetch_account: T) -> Vec<Result<(), Error>>
-		where T: Fn(&Address) -> AccountDetails { unimplemented!(); }
+	fn import_transactions<T>(&self, transactions: Vec<SignedTransaction>, _fetch_account: T) -> Vec<Result<(), Error>>
+		where T: Fn(&Address) -> AccountDetails {
+		// lets assume that all txs are valid
+		self.imported_transactions.lock().unwrap().extend_from_slice(&transactions);
+
+		transactions
+			.iter()
+			.map(|_| Ok(()))
+			.collect()
+	}
 
 	/// Returns hashes of transactions currently in pending
-	fn pending_transactions_hashes(&self) -> Vec<H256> { vec![] }
+	fn pending_transactions_hashes(&self) -> Vec<H256> {
+		vec![]
+	}
 
 	/// Removes all transactions from the queue and restart mining operation.
-	fn clear_and_reset(&self, _chain: &BlockChainClient) { unimplemented!(); }
+	fn clear_and_reset(&self, _chain: &BlockChainClient) {
+		unimplemented!();
+	}
 
 	/// Called when blocks are imported to chain, updates transactions queue.
-	fn chain_new_blocks(&self, _chain: &BlockChainClient, _imported: &[H256], _invalid: &[H256], _enacted: &[H256], _retracted: &[H256]) { unimplemented!(); }
+	fn chain_new_blocks(&self, _chain: &BlockChainClient, _imported: &[H256], _invalid: &[H256], _enacted: &[H256], _retracted: &[H256]) {
+		unimplemented!();
+	}
 
 	/// New chain head event. Restart mining operation.
-	fn update_sealing(&self, _chain: &BlockChainClient) { unimplemented!(); }
+	fn update_sealing(&self, _chain: &BlockChainClient) {
+		unimplemented!();
+	}
 
-	fn map_sealing_work<F, T>(&self, _chain: &BlockChainClient, _f: F) -> Option<T> where F: FnOnce(&ClosedBlock) -> T { unimplemented!(); }
+	fn map_sealing_work<F, T>(&self, _chain: &BlockChainClient, _f: F) -> Option<T> where F: FnOnce(&ClosedBlock) -> T {
+		unimplemented!();
+	}
 
 	fn transaction(&self, hash: &H256) -> Option<SignedTransaction> {
-		self.pending_transactions.lock().unwrap().get(hash).and_then(|tx_ref| Some(tx_ref.clone()))
+		self.pending_transactions.lock().unwrap().get(hash).cloned()
+	}
+
+	fn last_nonce(&self, address: &Address) -> Option<U256> {
+		self.last_nonces.read().unwrap().get(address).cloned()
 	}
 
 	/// Submit `seal` as a valid solution for the header of `pow_hash`.
 	/// Will check the seal, but not actually insert the block into the chain.
-	fn submit_seal(&self, _chain: &BlockChainClient, _pow_hash: H256, _seal: Vec<Bytes>) -> Result<(), Error> { unimplemented!(); }
+	fn submit_seal(&self, _chain: &BlockChainClient, _pow_hash: H256, _seal: Vec<Bytes>) -> Result<(), Error> {
+		unimplemented!();
+	}
 }

rpc/src/v1/tests/personal.rs

@@ -22,8 +22,7 @@ use util::numbers::*;
 use std::collections::*;
 
 fn accounts_provider() -> Arc<TestAccountProvider> {
-	let mut accounts = HashMap::new();
-	accounts.insert(Address::from(1), TestAccount::new("test"));
+	let accounts = HashMap::new();
 	let ap = TestAccountProvider::new(accounts);
 	Arc::new(ap)
 }
@@ -38,7 +37,11 @@ fn setup() -> (Arc<TestAccountProvider>, IoHandler) {
 
 #[test]
 fn accounts() {
-	let (_test_provider, io) = setup();
+	let (test_provider, io) = setup();
+	test_provider.accounts
+		.write()
+		.unwrap()
+		.insert(Address::from(1), TestAccount::new("test"));
 
 	let request = r#"{"jsonrpc": "2.0", "method": "personal_listAccounts", "params": [], "id": 1}"#;
 	let response = r#"{"jsonrpc":"2.0","result":["0x0000000000000000000000000000000000000001"],"id":1}"#;
@@ -49,11 +52,22 @@ fn accounts() {
 
 #[test]
 fn new_account() {
-	let (_test_provider, io) = setup();
-
+	let (test_provider, io) = setup();
 	let request = r#"{"jsonrpc": "2.0", "method": "personal_newAccount", "params": ["pass"], "id": 1}"#;
-	let response = r#"{"jsonrpc":"2.0","result":"0x0000000000000000000000000000000000000002","id":1}"#;
 
-	assert_eq!(io.handle_request(request), Some(response.to_owned()));
+	let res = io.handle_request(request);
+
+	let accounts = test_provider.accounts.read().unwrap();
+	assert_eq!(accounts.len(), 1);
+
+	let address = accounts
+		.keys()
+		.nth(0)
+		.cloned()
+		.unwrap();
+
+	let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"","id":1}"#;
+
+	assert_eq!(res, Some(response));
 }