merge fixes

ethcore/src/miner/miner.rs

@@ -18,7 +18,7 @@ use rayon::prelude::*;
 use std::sync::atomic::AtomicBool;
 
 use util::*;
-use util::keys::store::{AccountService, AccountProvider};
+use util::keys::store::{AccountProvider};
 use views::{BlockView, HeaderView};
 use client::{MiningBlockChainClient, BlockID};
 use block::{ClosedBlock, IsBlock};
@@ -44,7 +44,7 @@ pub struct Miner {
 	extra_data: RwLock<Bytes>,
 	spec: Spec,
 
-	accounts: RwLock<Option<Arc<AccountService>>>,		// TODO: this is horrible since AccountService already contains a single RwLock field. refactor.
+	accounts: Option<Arc<AccountProvider>>,
 }
 
 impl Default for Miner {
@@ -58,7 +58,7 @@ impl Default for Miner {
 			gas_floor_target: RwLock::new(U256::zero()),
 			author: RwLock::new(Address::default()),
 			extra_data: RwLock::new(Vec::new()),
-			accounts: RwLock::new(None),
+			accounts: None,
 			spec: Spec::new_test(),
 		}
 	}
@@ -76,13 +76,13 @@ impl Miner {
 			gas_floor_target: RwLock::new(U256::zero()),
 			author: RwLock::new(Address::default()),
 			extra_data: RwLock::new(Vec::new()),
-			accounts: RwLock::new(None),
+			accounts: None,
 			spec: spec,
 		})
 	}
 
 	/// Creates new instance of miner
-	pub fn with_accounts(force_sealing: bool, spec: Spec, accounts: Arc<AccountService>) -> Arc<Miner> {
+	pub fn with_accounts(force_sealing: bool, spec: Spec, accounts: Arc<AccountProvider>) -> Arc<Miner> {
 		Arc::new(Miner {
 			transaction_queue: Mutex::new(TransactionQueue::new()),
 			force_sealing: force_sealing,
@@ -92,7 +92,7 @@ impl Miner {
 			gas_floor_target: RwLock::new(U256::zero()),
 			author: RwLock::new(Address::default()),
 			extra_data: RwLock::new(Vec::new()),
-			accounts: RwLock::new(Some(accounts)),
+			accounts: Some(accounts),
 			spec: spec,
 		})
 	}
@@ -177,9 +177,8 @@ impl Miner {
 			if !block.transactions().is_empty() {
 				trace!(target: "miner", "prepare_sealing: block has transaction - attempting internal seal.");
 				// block with transactions - see if we can seal immediately.
-				let a = self.accounts.read().unwrap();
+				let s = self.engine().generate_seal(block.block(), match self.accounts {
-				let s = self.engine().generate_seal(block.block(), match *a.deref() {
+					Some(ref x) => Some(&**x),
-					Some(ref x) => Some(x.deref() as &AccountProvider),
 					None => None,
 				});
 				if let Some(seal) = s {

rpc/src/v1/tests/eth.rs

@@ -19,23 +19,58 @@ use std::collections::HashMap;
 use std::sync::Arc;
 use std::str::FromStr;
 
+<<<<<<< HEAD
 use ethcore::client::{MiningBlockChainClient, BlockChainClient, Client, ClientConfig};
 use ethcore::spec::Genesis;
+=======
+use ethcore::ids::BlockID;
+use ethcore::client::{Client, BlockChainClient, ClientConfig};
+use ethcore::spec::{Genesis, Spec};
 use ethcore::block::Block;
+use ethcore::views::BlockView;
 use ethcore::ethereum;
 use ethcore::transaction::{Transaction, Action};
 use ethcore::miner::{MinerService, ExternalMiner, Miner};
 use devtools::RandomTempPath;
+use util::Hashable;
 use util::io::IoChannel;
-use util::hash::Address;
+use util::hash::{Address, H256};
-use util::numbers::{Uint, U256};
+use util::numbers::U256;
 use util::keys::{AccountProvider, TestAccount, TestAccountProvider};
 use jsonrpc_core::IoHandler;
 use ethjson::blockchain::BlockChain;
 
 use v1::traits::eth::Eth;
 use v1::impls::EthClient;
-use v1::tests::helpers::{TestSyncProvider, Config, TestMinerService};
+use v1::tests::helpers::{TestSyncProvider, Config};
+
+fn account_provider() -> Arc<TestAccountProvider> {
+	let mut accounts = HashMap::new();
+	accounts.insert(Address::from(1), TestAccount::new("test"));
+	let ap = TestAccountProvider::new(accounts);
+	Arc::new(ap)
+}
+
+fn sync_provider() -> Arc<TestSyncProvider> {
+	Arc::new(TestSyncProvider::new(Config {
+		network_id: U256::from(3),
+		num_peers: 120,
+	}))
+}
+
+fn miner_service(spec: Spec, accounts: Arc<AccountProvider>) -> Arc<Miner> {
+	Miner::with_accounts(true, spec, accounts)
+}
+
+fn make_spec(chain: &BlockChain) -> Spec {
+	let genesis = Genesis::from(chain.genesis());
+	let mut spec = ethereum::new_frontier_test();
+	let state = chain.pre_state.clone().into();
+	spec.set_genesis_state(state);
+	spec.overwrite_genesis_params(genesis);
+	assert!(spec.is_state_root_valid());
+	spec
+}
 
 struct EthTester {
 	_client: Arc<MiningBlockChainClient>,
@@ -44,16 +79,65 @@ struct EthTester {
 	handler: IoHandler,
 }
 
+impl EthTester {
+	fn from_chain(chain: &BlockChain) -> Self {
+		let tester = Self::from_spec_provider(|| make_spec(chain));
+
+		for b in &chain.blocks_rlp() {
+			if Block::is_good(&b) {
+				let _ = tester.client.import_block(b.clone());
+				tester.client.flush_queue();
+				tester.client.import_verified_blocks(&IoChannel::disconnected());
+			}
+		}
+
+		tester.client.flush_queue();
+
+		assert!(tester.client.chain_info().best_block_hash == chain.best_block.clone().into());
+		tester
+	}
+
+	fn from_spec_provider<F>(spec_provider: F) -> Self
+		where F: Fn() -> Spec {
+
+		let dir = RandomTempPath::new();
+		let client = Client::new(ClientConfig::default(), spec_provider(), dir.as_path(), IoChannel::disconnected()).unwrap();
+		let sync_provider = sync_provider();
+		let account_provider = account_provider();
+		let miner_service = miner_service(spec_provider(), account_provider.clone());
+		let external_miner = Arc::new(ExternalMiner::default());
+
+		let eth_client = EthClient::new(
+			&client,
+			&sync_provider,
+			&account_provider,
+			&miner_service,
+			&external_miner
+		);
+
+		let handler = IoHandler::new();
+		let delegate = eth_client.to_delegate();
+		handler.add_delegate(delegate);
+
+		EthTester {
+			_miner: miner_service,
+			client: client,
+			accounts: account_provider,
+			handler: handler,
+		}
+	}
+}
+
 #[test]
 fn harness_works() {
 	let chain: BlockChain = extract_chain!("BlockchainTests/bcUncleTest");
-	chain_harness(chain, |_| {});
+	let _ = EthTester::from_chain(&chain);
 }
 
 #[test]
 fn eth_get_balance() {
 	let chain = extract_chain!("BlockchainTests/bcWalletTest", "wallet2outOf3txs");
-	chain_harness(chain, |tester| {
+	let tester = EthTester::from_chain(&chain);
 	// final account state
 	let req_latest = r#"{
 		"jsonrpc": "2.0",
@@ -74,13 +158,12 @@ fn eth_get_balance() {
 
 	let res_new_acc = r#"{"jsonrpc":"2.0","result":"0x00","id":3}"#.to_owned();
 	assert_eq!(tester.handler.handle_request(req_new_acc).unwrap(), res_new_acc);
-	});
 }
 
 #[test]
 fn eth_block_number() {
 	let chain = extract_chain!("BlockchainTests/bcRPC_API_Test");
-	chain_harness(chain, |tester| {
+	let tester = EthTester::from_chain(&chain);
 	let req_number = r#"{
 		"jsonrpc": "2.0",
 		"method": "eth_blockNumber",
@@ -90,16 +173,67 @@ fn eth_block_number() {
 
 	let res_number = r#"{"jsonrpc":"2.0","result":"0x20","id":1}"#.to_owned();
 	assert_eq!(tester.handler.handle_request(req_number).unwrap(), res_number);
-	});
 }
 
-#[cfg(test)]
+// a frontier-like test with an expanded gas limit and balance on known account.
+const TRANSACTION_COUNT_SPEC: &'static [u8] = br#"{
+	"name": "Frontier (Test)",
+	"engine": {
+		"Ethash": {
+			"params": {
+				"gasLimitBoundDivisor": "0x0400",
+				"minimumDifficulty": "0x020000",
+				"difficultyBoundDivisor": "0x0800",
+				"durationLimit": "0x0d",
+				"blockReward": "0x4563918244F40000",
+				"registrar" : "0xc6d9d2cd449a754c494264e1809c50e34d64562b",
+				"frontierCompatibilityModeLimit": "0xffffffffffffffff"
+			}
+		}
+	},
+	"params": {
+		"accountStartNonce": "0x00",
+		"maximumExtraDataSize": "0x20",
+		"minGasLimit": "0x50000",
+		"networkID" : "0x1"
+	},
+	"genesis": {
+		"seal": {
+			"ethereum": {
+				"nonce": "0x0000000000000042",
+				"mixHash": "0x0000000000000000000000000000000000000000000000000000000000000000"
+			}
+		},
+		"difficulty": "0x400000000",
+		"author": "0x0000000000000000000000000000000000000000",
+		"timestamp": "0x00",
+		"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
+		"extraData": "0x11bbe8db4e347b4e8c937c1c8370e4b5ed33adb3db69cbdb7a38e1e50b1b82fa",
+		"gasLimit": "0x50000"
+	},
+	"accounts": {
+		"0000000000000000000000000000000000000001": { "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } },
+		"0000000000000000000000000000000000000002": { "builtin": { "name": "sha256", "pricing": { "linear": { "base": 60, "word": 12 } } } },
+		"0000000000000000000000000000000000000003": { "builtin": { "name": "ripemd160", "pricing": { "linear": { "base": 600, "word": 120 } } } },
+		"0000000000000000000000000000000000000004": { "builtin": { "name": "identity", "pricing": { "linear": { "base": 15, "word": 3 } } } },
+		"faa34835af5c2ea724333018a515fbb7d5bc0b33": { "balance": "10000000000000", "nonce": "0" }
+	}
+}
+"#;
+
 #[test]
 fn eth_transaction_count() {
-	let chain = extract_chain!("BlockchainTests/bcRPC_API_Test");
+	use util::crypto::Secret;
-	chain_harness(chain, |tester| {
+
-		let address = tester.accounts.new_account("123").unwrap();
+	let address = Address::from_str("faa34835af5c2ea724333018a515fbb7d5bc0b33").unwrap();
-		let secret = tester.accounts.account_secret(&address).unwrap();
+	let secret = Secret::from_str("8a283037bb19c4fed7b1c569e40c7dcff366165eb869110a1b11532963eb9cb2").unwrap();
+
+	let tester = EthTester::from_spec_provider(|| Spec::load(TRANSACTION_COUNT_SPEC));
+	tester.accounts.accounts.write().unwrap().insert(address, TestAccount {
+		unlocked: false,
+		password: "123".into(),
+		secret: secret
+	});
 
 	let req_before = r#"{
 		"jsonrpc": "2.0",
@@ -112,32 +246,21 @@ fn eth_transaction_count() {
 
 	assert_eq!(tester.handler.handle_request(&req_before).unwrap(), res_before);
 
-		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 req_send_trans = r#"{
 		"jsonrpc": "2.0",
 		"method": "eth_sendTransaction",
 		"params": [{
 			"from": ""#.to_owned() + format!("0x{:?}", address).as_ref() + r#"",
 			"to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567",
-				"gas": "0x76c0",
+			"gas": "0x30000",
-				"gasPrice": "0x9184e72a000",
+			"gasPrice": "0x01",
 			"value": "0x9184e72a"
 		}],
 		"id": 16
 	}"#;
 
-		let res_send_trans = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", t.hash()).as_ref() + r#"","id":16}"#;
-
 	// dispatch the transaction.
-		assert_eq!(tester.handler.handle_request(&req_send_trans).unwrap(), res_send_trans);
+	tester.handler.handle_request(&req_send_trans).unwrap();
 
 	// we have submitted the transaction -- but this shouldn't be reflected in a "latest" query.
 	let req_after_latest = r#"{
@@ -162,38 +285,69 @@ fn eth_transaction_count() {
 	let res_after_pending = r#"{"jsonrpc":"2.0","result":"0x01","id":18}"#;
 
 	assert_eq!(&tester.handler.handle_request(&req_after_pending).unwrap(), res_after_pending);
-	});
 }
 
-fn account_provider() -> Arc<TestAccountProvider> {
+fn verify_transaction_counts(name: String, chain: BlockChain) {
-	let mut accounts = HashMap::new();
+	struct PanicHandler(String);
-	accounts.insert(Address::from(1), TestAccount::new("test"));
+	impl Drop for PanicHandler {
-	let ap = TestAccountProvider::new(accounts);
+		fn drop(&mut self) {
-	Arc::new(ap)
+			if ::std::thread::panicking() {
+				println!("Test failed: {}", self.0);
+			}
+		}
 	}
 
-fn sync_provider() -> Arc<TestSyncProvider> {
+	let _panic = PanicHandler(name);
-	Arc::new(TestSyncProvider::new(Config {
+
-		network_id: U256::from(3),
+	fn by_hash(hash: H256, count: usize, id: &mut usize) -> (String, String) {
-		num_peers: 120,
+		let req = r#"{
-	}))
+			"jsonrpc": "2.0",
+			"method": "eth_getBlockTransactionCountByHash",
+			"params": [
+				""#.to_owned() + format!("0x{:?}", hash).as_ref() + r#""
+			],
+			"id": "# + format!("{}", *id).as_ref() + r#"
+		}"#;
+
+		let res = r#"{"jsonrpc":"2.0","result":""#.to_owned()
+			+ format!("0x{:02x}", count).as_ref()
+			+ r#"","id":"#
+			+ format!("{}", *id).as_ref() + r#"}"#;
+		*id += 1;
+		(req, res)
 	}
 
-fn miner_service() -> Arc<TestMinerService> {
+	fn by_number(num: u64, count: usize, id: &mut usize) -> (String, String) {
-	Arc::new(TestMinerService::default())
+		let req = r#"{
+			"jsonrpc": "2.0",
+			"method": "eth_getBlockTransactionCountByNumber",
+			"params": [
+				"#.to_owned() + &::serde_json::to_string(&U256::from(num)).unwrap() + r#"
+			],
+			"id": "# + format!("{}", *id).as_ref() + r#"
+		}"#;
+
+		let res = r#"{"jsonrpc":"2.0","result":""#.to_owned()
+			+ format!("0x{:02x}", count).as_ref()
+			+ r#"","id":"#
+			+ format!("{}", *id).as_ref() + r#"}"#;
+		*id += 1;
+		(req, res)
 	}
 
-// given a blockchain, this harness will create an EthClient wrapping it
+	let tester = EthTester::from_chain(&chain);
-// which tests can pass specially crafted requests to.
-fn chain_harness<F, U>(chain: BlockChain, mut cb: F) -> U
-	where F: FnMut(&EthTester) -> U {
-	let genesis = Genesis::from(chain.genesis());
-	let mut spec = ethereum::new_frontier_test();
-	let state = chain.pre_state.clone().into();
-	spec.set_genesis_state(state);
-	spec.overwrite_genesis_params(genesis);
-	assert!(spec.is_state_root_valid());
 
+	let mut id = 1;
+	for b in chain.blocks_rlp().iter().filter(|b| Block::is_good(b)).map(|b| BlockView::new(b)) {
+		let count = b.transactions_count();
+
+		let hash = b.sha3();
+		let number = b.header_view().number();
+
+		let (req, res) = by_hash(hash, count, &mut id);
+		assert_eq!(tester.handler.handle_request(&req), Some(res));
+
+<<<<<<< HEAD
 	let dir = RandomTempPath::new();
 	let client = Client::new(ClientConfig::default(), spec, dir.as_path(), Arc::new(Miner::default()), IoChannel::disconnected()).unwrap();
 	let sync_provider = sync_provider();
@@ -206,24 +360,16 @@ fn chain_harness<F, U>(chain: BlockChain, mut cb: F) -> U
 			let _ = client.import_block(b.clone());
 			client.flush_queue();
 			client.import_verified_blocks(&IoChannel::disconnected());
+=======
+		// uncles can share block numbers, so skip them.
+		if tester.client.block_hash(BlockID::Number(number)) == Some(hash) {
+			let (req, res) = by_number(number, count, &mut id);
+			assert_eq!(tester.handler.handle_request(&req), Some(res));
+>>>>>>> master
+		}
 	}
 }
 
-	assert!(client.chain_info().best_block_hash == chain.best_block.into());
+register_test!(eth_transaction_count_1, verify_transaction_counts, "BlockchainTests/bcWalletTest");
-
+register_test!(eth_transaction_count_2, verify_transaction_counts, "BlockchainTests/bcTotalDifficultyTest");
-	let eth_client = EthClient::new(&client, &sync_provider, &account_provider,
+register_test!(eth_transaction_count_3, verify_transaction_counts, "BlockchainTests/bcGasPricerTest");
-		&miner_service, &external_miner);
-
-	let handler = IoHandler::new();
-	let delegate = eth_client.to_delegate();
-	handler.add_delegate(delegate);
-
-	let tester = EthTester {
-		_miner: miner_service,
-		_client: client,
-		accounts: account_provider,
-		handler: handler,
-	};
-
-	cb(&tester)
-}

rpc/src/v1/tests/mod.rs

@@ -13,11 +13,15 @@ pub mod helpers;
 //     extract the chain with that name. This will panic if no chain by that name
 //     is found.
 macro_rules! extract_chain {
-	($file:expr, $name:expr) => {{
+	(iter $file:expr) => {{
 		const RAW_DATA: &'static [u8] =
 			include_bytes!(concat!("../../../../ethcore/res/ethereum/tests/", $file, ".json"));
+		::ethjson::blockchain::Test::load(RAW_DATA).unwrap().into_iter()
+	}};
+
+	($file:expr, $name:expr) => {{
 		let mut chain = None;
-		for (name, c) in ::ethjson::blockchain::Test::load(RAW_DATA).unwrap() {
+		for (name, c) in extract_chain!(iter $file) {
 			if name == $name {
 				chain = Some(c);
 				break;
@@ -27,14 +31,41 @@ macro_rules! extract_chain {
 	}};
 
 	($file:expr) => {{
-		const RAW_DATA: &'static [u8] =
+		extract_chain!(iter $file).next().unwrap().1
-			include_bytes!(concat!("../../../../ethcore/res/ethereum/tests/", $file, ".json"));
-
-		::ethjson::blockchain::Test::load(RAW_DATA)
-			.unwrap().into_iter().next().unwrap().1
 	}};
 }
 
+macro_rules! register_test {
+	($name:ident, $cb:expr, $file:expr) => {
+		#[test]
+		fn $name() {
+			for (name, chain) in extract_chain!(iter $file) {
+				$cb(name, chain);
+			}
+		}
+	};
+
+	(heavy $name:ident, $cb:expr, $file:expr) => {
+		#[test]
+		#[cfg(feature = "test-heavy")]
+		fn $name() {
+			for (name, chain) in extract_chain!(iter $file) {
+				$cb(name, chain);
+			}
+		}
+	};
+
+	(ignore $name:ident, $cb:expr, $file:expr) => {
+		#[test]
+		#[ignore]
+		fn $name() {
+			for (name, chain) in extract_chain!(iter $file) {
+				$cb(name, chain);
+			}
+		}
+	};
+}
+
 #[cfg(test)]
 mod mocked;
 #[cfg(test)]

util/src/keys/store.rs

@@ -325,7 +325,7 @@ impl SecretStore {
 		ret
 	}
 
-	/// Returns secret for unlocked account.
+	/// Returns secret for locked account.
 	pub fn locked_account_secret(&self, account: &Address, pass: &str) -> Result<crypto::Secret, SigningError> {
 		let secret_id = try!(self.account(&account).ok_or(SigningError::NoAccount));
 		self.get(&secret_id, pass).or_else(|e| Err(match e {