refactor: Move run_transaction params into fields of a TxInput struct and make doc comments of its fields

ethcore/src/client/evm_test_client.rs

@@ -85,7 +85,7 @@ impl<'a> fmt::Debug for EvmTestClient<'a> {
 
 impl<'a> EvmTestClient<'a> {
 	/// Converts a json spec definition into spec.
-	pub fn spec_from_json(spec: &ForkSpec) -> Option<spec::Spec> {
+	pub fn fork_spec_from_json(spec: &ForkSpec) -> Option<spec::Spec> {
 		match *spec {
 			ForkSpec::Frontier => Some(ethereum::new_frontier_test()),
 			ForkSpec::Homestead => Some(ethereum::new_homestead_test()),

ethcore/src/json_tests/chain.rs

@@ -65,7 +65,7 @@ pub fn json_chain_test<H: FnMut(&str, HookType)>(json_data: &[u8], start_stop_ho
 			flush!("   - {}...", name);
 
 			let spec = {
-				let mut spec = match EvmTestClient::spec_from_json(&blockchain.network) {
+				let mut spec = match EvmTestClient::fork_spec_from_json(&blockchain.network) {
 					Some(spec) => spec,
 					None => {
 						println!("   - {} | {:?} Ignoring tests because of missing spec", name, blockchain.network);

ethcore/src/json_tests/state.rs

@@ -62,7 +62,7 @@ pub fn json_chain_test<H: FnMut(&str, HookType)>(json_data: &[u8], start_stop_ho
 
 			for (spec_name, states) in test.post_states {
 				let total = states.len();
-				let spec = match EvmTestClient::spec_from_json(&spec_name) {
+				let spec = match EvmTestClient::fork_spec_from_json(&spec_name) {
 					Some(spec) => spec,
 					None => {
 						println!("   - {} | {:?} Ignoring tests because of missing spec", name, spec_name);

ethcore/src/json_tests/transaction.rs

@@ -44,7 +44,7 @@ fn do_json_test<H: FnMut(&str, HookType)>(json_data: &[u8], start_stop_hook: &mu
 		start_stop_hook(&name, HookType::OnStart);
 
 		for (spec_name, result) in test.post_state {
-			let spec = match EvmTestClient::spec_from_json(&spec_name) {
+			let spec = match EvmTestClient::fork_spec_from_json(&spec_name) {
 				Some(spec) => spec,
 				None => {
 					println!("   - {} | {:?} Ignoring tests because of missing spec", name, spec_name);

ethcore/types/src/transaction/transaction.rs

@@ -319,7 +319,7 @@ impl UnverifiedTransaction {
 		self
 	}
 
-	/// Checks is signature is empty.
+	/// Checks if the signature is empty.
 	pub fn is_unsigned(&self) -> bool {
 		self.r.is_zero() && self.s.is_zero()
 	}

evmbin/README.md

@@ -30,7 +30,7 @@ Transaction options:
     --gas-price WEI    Supplied gas price as hex (without 0x).
 
 State test options:
-    --only NAME        Runs only a single test matching the name.
+    --only NAME        Runs only a single state test matching the name.
     --chain CHAIN      Run only tests from specific chain.
 
 General options:

evmbin/src/info.rs

@@ -100,44 +100,61 @@ pub fn run_action<T: Informant>(
 	})
 }
 
+/// Input data to run transaction.
+#[derive(Debug)]
+pub struct TxInput<'a, T> {
+	/// State test name associated with the transaction.
+	pub state_test_name: &'a str,
+	/// Transaction index from list of transactions within a state root hash corresponding to a chain.
+	pub tx_index: usize,
+	/// Fork specification (i.e. Constantinople, EIP150, EIP158, etc).
+	pub fork_spec_name: &'a ethjson::spec::ForkSpec,
+	/// State of all accounts in the system that is a binary tree mapping of each account address to account data
+	/// that is expressed as Plain Old Data containing the account balance, account nonce, account code in bytes,
+	/// and the account storage binary tree map.
+	pub pre_state: &'a pod_state::PodState,
+	/// State root hash associated with the transaction.
+	pub post_root: H256,
+	/// Client environment information associated with the transaction's chain specification.
+	pub env_info: &'a client::EnvInfo,
+	/// Signed transaction accompanied by a signature that may be unverified and a successfully recovered
+	/// sender address. The unverified transaction contains a recoverable ECDSA signature that has been encoded
+	/// as RSV components and includes replay protection for the specified chain. Verification of the signed transaction
+	/// with a valid secret of an account's keypair and a specific chain may be used to recover the sender's public key
+	/// and their associated address by applying the Keccak-256 hash function.
+	pub transaction: transaction::SignedTransaction,
+	/// JSON formatting informant.
+	pub informant: T,
+	///	Trie specification (i.e. Generic trie, Secure trie, Secure with fat database).
+	pub trie_spec: TrieSpec,
+}
+
 /// Execute given transaction and verify resulting state root.
 pub fn run_transaction<T: Informant>(
-	// Chain specification name associated with the transaction.
-	name: &str,
-	// Transaction index from list of transactions within a state root hashes corresponding to a chain.
-	idx: usize,
-	// Fork specification (i.e. Constantinople, EIP150, EIP158, etc).
-	spec: &ethjson::spec::ForkSpec,
-	// State of all accounts in the system that is a binary tree mapping of each account address to account data
-	// that is expressed as Plain Old Data containing the account balance, account nonce, account code in bytes,
-	// and the account storage binary tree map.
-	pre_state: &pod_state::PodState,
-	// State root hash associated with the transaction.
-	post_root: H256,
-	// Client environment information associated with the transaction's chain specification.
-	env_info: &client::EnvInfo,
-	transaction: transaction::SignedTransaction,
-	// JSON formatting informant.
-	mut informant: T,
-	trie_spec: TrieSpec,
+	tx_input: TxInput<T>
 ) {
-	let spec_name = format!("{:?}", spec).to_lowercase();
-	let spec = match EvmTestClient::spec_from_json(spec) {
+	let TxInput {
+		state_test_name, tx_index, fork_spec_name, pre_state, post_root, env_info, transaction, mut informant, trie_spec, ..
+	} = tx_input;
+	let fork_spec_name_formatted = format!("{:?}", fork_spec_name).to_lowercase();
+	let fork_spec = match EvmTestClient::fork_spec_from_json(&fork_spec_name) {
 		Some(spec) => {
-			informant.before_test(&format!("{}:{}:{}", name, spec_name, idx), "starting");
+			informant.before_test(
+				&format!("{}:{}:{}", &state_test_name, &fork_spec_name_formatted, tx_index), "starting");
 			spec
 		},
 		None => {
-			informant.before_test(&format!("{}:{}:{}", name, spec_name, idx), "skipping because of missing spec");
+			informant.before_test(&format!("{}:{}:{}",
+				&state_test_name, fork_spec_name_formatted, &tx_index), "skipping because of missing fork specification");
 			return;
 		},
 	};
 
 	informant.set_gas(env_info.gas_limit);
 
 	let mut sink = informant.clone_sink();
-	let result = run(&spec, trie_spec, transaction.gas, pre_state, |mut client| {
-		let result = client.transact(env_info, transaction, trace::NoopTracer, informant);
+	let result = run(&fork_spec, trie_spec, transaction.gas, &pre_state, |mut client| {
+		let result = client.transact(&env_info, transaction, trace::NoopTracer, informant);
 		match result {
 			Ok(TransactSuccess { state_root, gas_left, output, vm_trace, end_state, .. }) => {
 				if state_root != post_root {

evmbin/src/main.rs

@@ -71,7 +71,7 @@ use vm::{ActionParams, CallType};
 mod info;
 mod display;
 
-use info::Informant;
+use info::{Informant, TxInput};
 
 const USAGE: &'static str = r#"
 EVM implementation for Parity.
@@ -162,10 +162,10 @@ fn run_state_test(args: Args) {
 
 	// Iterate over 1st level (outer) key-value pair of the state test JSON file.
 	// Skip to next iteration if CLI option `--only NAME` was parsed into `only_test` and does not match
-	// the current key `name` (i.e. add11, create2callPrecompiles).
-	for (name, test) in state_test {
+	// the current key `state_test_name` (i.e. add11, create2callPrecompiles).
+	for (state_test_name, test) in state_test {
 		if let Some(false) = only_test.as_ref().map(|only_test| {
-			&name.to_lowercase() == only_test
+			&state_test_name.to_lowercase() == only_test
 		}) {
 			continue;
 		}
@@ -177,17 +177,17 @@ fn run_state_test(args: Args) {
 
 		// Iterate over remaining "post" key of the 2nd level key-value pairs in the state test JSON file.
 		// Skip to next iteration if CLI option `--chain CHAIN` was parsed into `only_chain` and does not match
-		// the current key `spec` (i.e. Constantinople, EIP150, EIP158).
-		for (spec, states) in test.post_states {
+		// the current key `fork_spec_name` (i.e. Constantinople, EIP150, EIP158).
+		for (fork_spec_name, states) in test.post_states {
 			if let Some(false) = only_chain.as_ref().map(|only_chain| {
-				&format!("{:?}", spec).to_lowercase() == only_chain
+				&format!("{:?}", fork_spec_name).to_lowercase() == only_chain
 			}) {
 				continue;
 			}
 
 			// Iterate over the 3rd level key-value pairs of the state test JSON file
 			// (i.e. list of transactions and associated state roots hashes corresponding each chain).
-			for (idx, state) in states.into_iter().enumerate() {
+			for (tx_index, state) in states.into_iter().enumerate() {
 				let post_root = state.hash.into();
 				let transaction = multitransaction.select(&state.indexes).into();
 
@@ -206,24 +206,79 @@ fn run_state_test(args: Args) {
 				// for CLI option `--std-dump-json` or `--std-json`.
 				if args.flag_std_dump_json || args.flag_std_json {
 					if args.flag_std_err_only {
+						let tx_input = TxInput {
+							state_test_name: &state_test_name,
+							tx_index,
+							fork_spec_name: &fork_spec_name,
+							pre_state: &pre,
+							post_root,
+							env_info: &env_info,
+							transaction,
+							informant: display::std_json::Informant::err_only(),
+							trie_spec,
+						};
 						// Use Standard JSON informant with err only
-						info::run_transaction(&name, idx, &spec, &pre, post_root, &env_info, transaction, display::std_json::Informant::err_only(), trie_spec)
+						info::run_transaction(tx_input)
 					} else if args.flag_std_out_only {
+						let tx_input = TxInput {
+							state_test_name: &state_test_name,
+							tx_index,
+							fork_spec_name: &fork_spec_name,
+							pre_state: &pre,
+							post_root,
+							env_info: &env_info,
+							transaction,
+							informant: display::std_json::Informant::out_only(),
+							trie_spec,
+						};
 						// Use Standard JSON informant with out only
-						info::run_transaction(&name, idx, &spec, &pre, post_root, &env_info, transaction, display::std_json::Informant::out_only(), trie_spec)
+						info::run_transaction(tx_input)
 					} else {
+						let tx_input = TxInput {
+							state_test_name: &state_test_name,
+							tx_index,
+							fork_spec_name: &fork_spec_name,
+							pre_state: &pre,
+							post_root,
+							env_info: &env_info,
+							transaction,
+							informant: display::std_json::Informant::default(),
+							trie_spec,
+						};
 						// Use Standard JSON informant default
-						info::run_transaction(&name, idx, &spec, &pre, post_root, &env_info, transaction, display::std_json::Informant::default(), trie_spec)
+						info::run_transaction(tx_input)
 					}
 				} else {
 					// Execute the given transaction and verify resulting state root
 					// for CLI option `--json`.
 					if args.flag_json {
+						let tx_input = TxInput {
+							state_test_name: &state_test_name,
+							tx_index,
+							fork_spec_name: &fork_spec_name,
+							pre_state: &pre,
+							post_root,
+							env_info: &env_info,
+							transaction,
+							informant: display::json::Informant::default(),
+							trie_spec,
+						};
 						// Use JSON informant
-						info::run_transaction(&name, idx, &spec, &pre, post_root, &env_info, transaction, display::json::Informant::default(), trie_spec)
+						info::run_transaction(tx_input)
 					} else {
+						let tx_input = TxInput {
+							state_test_name: &state_test_name,
+							tx_index,
+							fork_spec_name: &fork_spec_name,
+							pre_state: &pre,
+							post_root,
+							env_info: &env_info,
+							transaction,
+							informant: display::simple::Informant::default(),
+							trie_spec,
+						};
 						// Use Simple informant
-						info::run_transaction(&name, idx, &spec, &pre, post_root, &env_info, transaction, display::simple::Informant::default(), trie_spec)
+						info::run_transaction(tx_input)
 					}
 				}
 			}
@@ -417,6 +472,7 @@ mod tests {
 	use types::transaction;
 
 	use info;
+	use info::{TxInput};
 	use display;
 
 	#[derive(Debug, PartialEq, Deserialize)]
@@ -523,21 +579,32 @@ mod tests {
 			.expect("Serialization cannot fail; qed");
 
 		// Simulate the name CLI option `--only NAME`
-		let name = "add11".to_string();
-		let idx = 1;
+		let state_test_name = "add11".to_string();
+		let tx_index = 1;
 		// Simulate the chain `--chain CHAIN`
-		let spec = ForkSpec::EIP150;
+		let fork_spec_name = ForkSpec::EIP150;
 		let pre = _deserialized_state_tests.add11.pre_state.into();
 		let env_info = _deserialized_state_tests.add11.env.into();
 		let multitransaction = _deserialized_state_tests.add11.transaction;
-		for (spec, states) in _deserialized_state_tests.add11.post_states {
-			for (idx, state) in states.into_iter().enumerate() {
+		for (fork_spec_name, tx_states) in _deserialized_state_tests.add11.post_states {
+			for (tx_index, tx_state) in tx_states.into_iter().enumerate() {
 				let informant = display::json::Informant::default();
 				// Hash of latest transaction index in the chain
 				let post_root = H256::from_str("99a450d8ce5b987a71346d8a0a1203711f770745c7ef326912e46761f14cd764").unwrap();
 				let trie_spec = TrieSpec::Secure; // TrieSpec::Fat for --std_dump_json
-				let transaction: transaction::SignedTransaction = multitransaction.select(&state.indexes).into();
-				info::run_transaction(&name, idx, &spec, &pre, post_root, &env_info, transaction, informant, trie_spec)
+				let transaction: transaction::SignedTransaction = multitransaction.select(&tx_state.indexes).into();
+				let tx_input = TxInput {
+					state_test_name: &state_test_name,
+					tx_index,
+					fork_spec_name: &fork_spec_name,
+					pre_state: &pre,
+					post_root,
+					env_info: &env_info,
+					transaction,
+					informant,
+					trie_spec,
+				};
+				info::run_transaction(tx_input)
 			}
 		}
 	}
@@ -558,21 +625,32 @@ mod tests {
 			.expect("Serialization cannot fail; qed");
 
 		// Simulate the name CLI option `--only NAME`
-		let name = "create2callPrecompiles".to_string();
-		let idx = 7;
+		let state_test_name = "create2callPrecompiles".to_string();
+		let tx_index = 7;
 		// Simulate the chain `--chain CHAIN`
-		let spec = ForkSpec::Constantinople;
+		let fork_spec_name = ForkSpec::Constantinople;
 		let pre = _deserialized_state_tests.create2callPrecompiles.pre_state.into();
 		let env_info = _deserialized_state_tests.create2callPrecompiles.env.into();
 		let multitransaction = _deserialized_state_tests.create2callPrecompiles.transaction;
-		for (spec, states) in _deserialized_state_tests.create2callPrecompiles.post_states {
-			for (idx, state) in states.into_iter().enumerate() {
+		for (fork_spec_name, tx_states) in _deserialized_state_tests.create2callPrecompiles.post_states {
+			for (tx_index, tx_state) in tx_states.into_iter().enumerate() {
 				let informant = display::json::Informant::default();
 				// Hash of latest transaction index in the chain
 				let post_root = H256::from_str("0xde1d3953b508913c6e3e9bd412cd50daf60bb177517e5d1e8ccb0dab193aed03").unwrap();
 				let trie_spec = TrieSpec::Secure; // TrieSpec::Fat for --std_dump_json
-				let transaction: transaction::SignedTransaction = multitransaction.select(&state.indexes).into();
-				info::run_transaction(&name, idx, &spec, &pre, post_root, &env_info, transaction, informant, trie_spec)
+				let transaction: transaction::SignedTransaction = multitransaction.select(&tx_state.indexes).into();
+				let tx_input = TxInput {
+					state_test_name: &state_test_name,
+					tx_index,
+					fork_spec_name: &fork_spec_name,
+					pre_state: &pre,
+					post_root,
+					env_info: &env_info,
+					transaction,
+					informant,
+					trie_spec,
+				};
+				info::run_transaction(tx_input)
 			}
 		}
 	}