Update state tests execution model (#9440)

* Update & fix JSON state tests.

* Update tests to be able to run ethtest at
021fe3d410773024cd5f0387e62db6e6ec800f32.

- Touch user in state
- Adjust transaction tests to new json format

* Switch to same commit for submodule ethereum/test as geth (next includes constantinople changes).
Added test `json_tests::trie::generic::TrieTests_trieanyorder` and a few
difficulty tests.

* Remove trietestnextprev as it would require to parse differently and
implement it.

* Support new (shitty) format of transaction tests.

* Ignore junk in ethereum/tests repo.

* Ignore incorrect test.

* Update to a later commit

* Move block number to a constant.

* Fix ZK2 test - touched account should also be cleared.

* Fix conflict resolution

ethcore/res/ethereum/eip161_test.json

@@ -23,7 +23,7 @@
 		"eip161abcTransition": "0x0",
 		"eip161dTransition": "0x0",
 		"eip98Transition": "0x7fffffffffffffff",
-		"eip155Transition": "0x7fffffffffffffff",
+		"eip155Transition": "0x0",
 		"maxCodeSize": 24576,
 		"maxCodeSizeTransition": "0x0"
 	},

ethcore/src/client/evm_test_client.rs

@@ -60,7 +60,7 @@ impl fmt::Display for EvmTestError {
 }
 
 use ethereum;
-use ethjson::state::test::ForkSpec;
+use ethjson::spec::ForkSpec;
 
 /// Simplified, single-block EVM test client.
 pub struct EvmTestClient<'a> {
@@ -69,12 +69,12 @@ pub struct EvmTestClient<'a> {
 }
 
 impl<'a> fmt::Debug for EvmTestClient<'a> {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        fmt.debug_struct("EvmTestClient")
-            .field("state", &self.state)
-            .field("spec", &self.spec.name)
-            .finish()
-    }
+	fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+		fmt.debug_struct("EvmTestClient")
+			.field("state", &self.state)
+			.field("spec", &self.spec.name)
+			.finish()
+	}
 }
 
 impl<'a> EvmTestClient<'a> {
@@ -217,9 +217,27 @@ impl<'a> EvmTestClient<'a> {
 		let result = self.state.apply_with_tracing(&env_info, self.spec.engine.machine(), &transaction, tracer, vm_tracer);
 		let scheme = self.spec.engine.machine().create_address_scheme(env_info.number);
 
+		// Touch the coinbase at the end of the test to simulate
+		// miner reward.
+		// Details: https://github.com/paritytech/parity-ethereum/issues/9431
+		let schedule = self.spec.engine.machine().schedule(env_info.number);
+		self.state.add_balance(&env_info.author, &0.into(), if schedule.no_empty {
+			state::CleanupMode::NoEmpty
+		} else {
+			state::CleanupMode::ForceCreate
+		}).ok();
+		// Touching also means that we should remove the account if it's within eip161
+		// conditions.
+		self.state.kill_garbage(
+			&vec![env_info.author].into_iter().collect(),
+			schedule.kill_empty,
+			&None,
+			false
+		).ok();
+		self.state.commit().ok();
+
 		match result {
 			Ok(result) => {
-				self.state.commit().ok();
 				TransactResult::Ok {
 					state_root: *self.state.root(),
 					gas_left: initial_gas - result.receipt.gas_used,

ethcore/src/json_tests/difficulty.rs

@@ -52,27 +52,46 @@ pub fn json_difficulty_test<H: FnMut(&str, HookType)>(json_data: &[u8], spec: Sp
 	vec![]
 }
 
-mod difficulty_test_byzantium {
+macro_rules! difficulty_json_test {
+	( $spec:ident ) => {
+
 	use super::json_difficulty_test;
+	use tempdir::TempDir;
 	use json_tests::HookType;
 
 	fn do_json_test<H: FnMut(&str, HookType)>(json_data: &[u8], h: &mut H) -> Vec<String> {
-		json_difficulty_test(json_data, ::ethereum::new_byzantium_test(), h)
+		let tempdir = TempDir::new("").unwrap();
+		json_difficulty_test(json_data, ::ethereum::$spec(&tempdir.path()), h)
 	}
 
-	declare_test!{DifficultyTests_difficultyByzantium, "BasicTests/difficultyByzantium.json"}
+	}
 }
 
-mod difficulty_test_foundation {
+macro_rules! difficulty_json_test_nopath {
+	( $spec:ident ) => {
+
 	use super::json_difficulty_test;
-	use tempdir::TempDir;
 	use json_tests::HookType;
 
 	fn do_json_test<H: FnMut(&str, HookType)>(json_data: &[u8], h: &mut H) -> Vec<String> {
-		let tempdir = TempDir::new("").unwrap();
-		json_difficulty_test(json_data, ::ethereum::new_foundation(&tempdir.path()), h)
+		json_difficulty_test(json_data, ::ethereum::$spec(), h)
 	}
 
+	}
+}
+
+mod difficulty_test {
+	difficulty_json_test!(new_foundation);
+	declare_test!{DifficultyTests_difficulty, "BasicTests/difficulty.json"}
+}
+
+mod difficulty_test_byzantium {
+	difficulty_json_test_nopath!(new_byzantium_test);
+	declare_test!{DifficultyTests_difficultyByzantium, "BasicTests/difficultyByzantium.json"}
+}
+
+mod difficulty_test_foundation {
+	difficulty_json_test!(new_foundation);
 	declare_test!{DifficultyTests_difficultyMainNetwork, "BasicTests/difficultyMainNetwork.json"}
 }
 

ethcore/src/json_tests/state.rs

@@ -114,12 +114,16 @@ mod state_tests {
 		json_chain_test(json_data, h)
 	}
 
+	declare_test!{GeneralStateTest_stArgsZeroOneBalance, "GeneralStateTests/stArgsZeroOneBalance/"}
 	declare_test!{GeneralStateTest_stAttackTest, "GeneralStateTests/stAttackTest/"}
 	declare_test!{GeneralStateTest_stBadOpcodeTest, "GeneralStateTests/stBadOpcode/"}
+	declare_test!{GeneralStateTest_stBugs, "GeneralStateTests/stBugs/"}
 	declare_test!{GeneralStateTest_stCallCodes, "GeneralStateTests/stCallCodes/"}
+	declare_test!{GeneralStateTest_stCallCreateCallCodeTest, "GeneralStateTests/stCallCreateCallCodeTest/"}
 	declare_test!{GeneralStateTest_stCallDelegateCodesCallCodeHomestead, "GeneralStateTests/stCallDelegateCodesCallCodeHomestead/"}
 	declare_test!{GeneralStateTest_stCallDelegateCodesHomestead, "GeneralStateTests/stCallDelegateCodesHomestead/"}
 	declare_test!{GeneralStateTest_stChangedEIP150, "GeneralStateTests/stChangedEIP150/"}
+	declare_test!{GeneralStateTest_stCodeCopyTest, "GeneralStateTests/stCodeCopyTest/"}
 	declare_test!{GeneralStateTest_stCodeSizeLimit, "GeneralStateTests/stCodeSizeLimit/"}
 	declare_test!{GeneralStateTest_stCreateTest, "GeneralStateTests/stCreateTest/"}
 	declare_test!{GeneralStateTest_stDelegatecallTestHomestead, "GeneralStateTests/stDelegatecallTestHomestead/"}
@@ -135,12 +139,15 @@ mod state_tests {
 	declare_test!{GeneralStateTest_stMemoryTest, "GeneralStateTests/stMemoryTest/"}
 	declare_test!{GeneralStateTest_stNonZeroCallsTest, "GeneralStateTests/stNonZeroCallsTest/"}
 	declare_test!{GeneralStateTest_stPreCompiledContracts, "GeneralStateTests/stPreCompiledContracts/"}
+	declare_test!{GeneralStateTest_stPreCompiledContracts2, "GeneralStateTests/stPreCompiledContracts2/"}
 	declare_test!{heavy => GeneralStateTest_stQuadraticComplexityTest, "GeneralStateTests/stQuadraticComplexityTest/"}
 	declare_test!{GeneralStateTest_stRandom, "GeneralStateTests/stRandom/"}
+	declare_test!{GeneralStateTest_stRandom2, "GeneralStateTests/stRandom2/"}
 	declare_test!{GeneralStateTest_stRecursiveCreate, "GeneralStateTests/stRecursiveCreate/"}
 	declare_test!{GeneralStateTest_stRefundTest, "GeneralStateTests/stRefundTest/"}
 	declare_test!{GeneralStateTest_stReturnDataTest, "GeneralStateTests/stReturnDataTest/"}
 	declare_test!{GeneralStateTest_stRevertTest, "GeneralStateTests/stRevertTest/"}
+	declare_test!{GeneralStateTest_stShift, "GeneralStateTests/stShift/"}
 	declare_test!{GeneralStateTest_stSolidityTest, "GeneralStateTests/stSolidityTest/"}
 	declare_test!{GeneralStateTest_stSpecialTest, "GeneralStateTests/stSpecialTest/"}
 	declare_test!{GeneralStateTest_stStackTests, "GeneralStateTests/stStackTests/"}
@@ -152,4 +159,11 @@ mod state_tests {
 	declare_test!{GeneralStateTest_stZeroCallsRevert, "GeneralStateTests/stZeroCallsRevert/"}
 	declare_test!{GeneralStateTest_stZeroCallsTest, "GeneralStateTests/stZeroCallsTest/"}
 	declare_test!{GeneralStateTest_stZeroKnowledge, "GeneralStateTests/stZeroKnowledge/"}
+
+	// Attempts to send a transaction that requires more than current balance:
+	// Tx:
+	// https://github.com/ethereum/tests/blob/726b161ba8a739691006cc1ba080672bb50a9d49/GeneralStateTests/stZeroKnowledge2/ecmul_0-3_5616_28000_96.json#L170
+	// Balance:
+	// https://github.com/ethereum/tests/blob/726b161ba8a739691006cc1ba080672bb50a9d49/GeneralStateTests/stZeroKnowledge2/ecmul_0-3_5616_28000_96.json#L126
+	declare_test!{GeneralStateTest_stZeroKnowledge2, "GeneralStateTests/stZeroKnowledge2/"}
 }

ethcore/src/json_tests/test_common.rs

@@ -41,6 +41,7 @@ pub fn run_test_path<H: FnMut(&str, HookType)>(
 		os.push(".json");
 		os
 	}).collect();
+	let extension = path.extension().and_then(|s| s.to_str());
 	if path.is_dir() {
 		for p in read_dir(path).unwrap().filter_map(|e| {
 			let e = e.unwrap();
@@ -51,6 +52,8 @@ pub fn run_test_path<H: FnMut(&str, HookType)>(
 			}}) {
 			run_test_path(&p, skip, runner, start_stop_hook)
 		}
+	} else if extension == Some("swp") || extension == None {
+		// Ignore junk
 	} else {
 		let mut path = p.to_path_buf();
 		path.set_extension("json");
@@ -64,7 +67,10 @@ pub fn run_test_file<H: FnMut(&str, HookType)>(
 	start_stop_hook: &mut H
 ) {
 	let mut data = Vec::new();
-	let mut file = File::open(&path).expect("Error opening test file");
+	let mut file = match File::open(&path) {
+		Ok(file) => file,
+		Err(_) => panic!("Error opening test file at: {:?}", path),
+	};
 	file.read_to_end(&mut data).expect("Error reading test file");
 	let results = runner(&data, start_stop_hook);
 	let empty: [String; 0] = [];

ethcore/src/json_tests/transaction.rs

@@ -16,10 +16,11 @@
 
 use std::path::Path;
 use super::test_common::*;
-use evm;
+use client::EvmTestClient;
+use header::Header;
 use ethjson;
 use rlp::Rlp;
-use transaction::{Action, UnverifiedTransaction, SignedTransaction};
+use transaction::UnverifiedTransaction;
 
 /// Run transaction jsontests on a given folder.
 pub fn run_test_path<H: FnMut(&str, HookType)>(p: &Path, skip: &[&'static str], h: &mut H) {
@@ -31,55 +32,61 @@ pub fn run_test_file<H: FnMut(&str, HookType)>(p: &Path, h: &mut H) {
 	::json_tests::test_common::run_test_file(p, do_json_test, h)
 }
 
+// Block number used to run the tests.
+// Make sure that all the specified features are activated.
+const BLOCK_NUMBER: u64 = 0x6ffffffffffffe;
+
 fn do_json_test<H: FnMut(&str, HookType)>(json_data: &[u8], start_stop_hook: &mut H) -> Vec<String> {
 	let tests = ethjson::transaction::Test::load(json_data).unwrap();
 	let mut failed = Vec::new();
-	let frontier_schedule = evm::Schedule::new_frontier();
-	let homestead_schedule = evm::Schedule::new_homestead();
-	let byzantium_schedule = evm::Schedule::new_byzantium();
 	for (name, test) in tests.into_iter() {
 		start_stop_hook(&name, HookType::OnStart);
 
-		let mut fail_unless = |cond: bool, title: &str| if !cond { failed.push(name.clone()); println!("Transaction failed: {:?}: {:?}", name, title); };
-
-		let number: Option<u64> = test.block_number.map(Into::into);
-		let schedule = match number {
-			None => &frontier_schedule,
-			Some(x) if x < 1_150_000 => &frontier_schedule,
-			Some(x) if x < 3_000_000 => &homestead_schedule,
-			Some(_) => &byzantium_schedule
-		};
-		let allow_chain_id_of_one = number.map_or(false, |n| n >= 2_675_000);
-		let allow_unsigned = number.map_or(false, |n| n >= 3_000_000);
-
-		let rlp: Vec<u8> = test.rlp.into();
-		let res = Rlp::new(&rlp)
-			.as_val()
-			.map_err(::error::Error::from)
-			.and_then(|t: UnverifiedTransaction| {
-				t.validate(schedule, schedule.have_delegate_call, allow_chain_id_of_one, allow_unsigned).map_err(Into::into)
-			});
-
-		fail_unless(test.transaction.is_none() == res.is_err(), "Validity different");
-		if let (Some(tx), Some(sender)) = (test.transaction, test.sender) {
-			let t = res.unwrap();
-			fail_unless(SignedTransaction::new(t.clone()).unwrap().sender() == sender.into(), "sender mismatch");
-			let is_acceptable_chain_id = match t.chain_id() {
-				None => true,
-				Some(1) if allow_chain_id_of_one => true,
-				_ => false,
+		for (spec_name, result) in test.post_state {
+			let spec = match EvmTestClient::spec_from_json(&spec_name) {
+				Some(spec) => spec,
+				None => {
+					println!("   - {} | {:?} Ignoring tests because of missing spec", name, spec_name);
+					continue;
+				}
+			};
+
+			let mut fail_unless = |cond: bool, title: &str| if !cond {
+				failed.push(format!("{}-{:?}", name, spec_name));
+				println!("Transaction failed: {:?}-{:?}: {:?}", name, spec_name, title);
 			};
-			fail_unless(is_acceptable_chain_id, "Network ID unacceptable");
-			let data: Vec<u8> = tx.data.into();
-			fail_unless(t.data == data, "data mismatch");
-			fail_unless(t.gas_price == tx.gas_price.into(), "gas_price mismatch");
-			fail_unless(t.nonce == tx.nonce.into(), "nonce mismatch");
-			fail_unless(t.value == tx.value.into(), "value mismatch");
-			let to: Option<ethjson::hash::Address> = tx.to.into();
-			let to: Option<Address> = to.map(Into::into);
-			match t.action {
-				Action::Call(dest) => fail_unless(Some(dest) == to, "call/destination mismatch"),
-				Action::Create => fail_unless(None == to, "create mismatch"),
+
+			let rlp: Vec<u8> = test.rlp.clone().into();
+			let res = Rlp::new(&rlp)
+				.as_val()
+				.map_err(::error::Error::from)
+				.and_then(|t: UnverifiedTransaction| {
+					let mut header: Header = Default::default();
+					// Use high enough number to activate all required features.
+					header.set_number(BLOCK_NUMBER);
+
+					let minimal = t.gas_required(&spec.engine.schedule(header.number())).into();
+					if t.gas < minimal {
+						return Err(::transaction::Error::InsufficientGas {
+							minimal, got: t.gas,
+						}.into());
+					}
+					spec.engine.verify_transaction_basic(&t, &header)?;
+					Ok(spec.engine.verify_transaction_unordered(t, &header)?)
+				});
+
+			match (res, result.hash, result.sender) {
+				(Ok(t), Some(hash), Some(sender)) => {
+					fail_unless(t.sender() == sender.into(), "sender mismatch");
+					fail_unless(t.hash() == hash.into(), "hash mismatch");
+				},
+				(Err(_), None, None) => {},
+				data => {
+					fail_unless(
+						false,
+						&format!("Validity different: {:?}", data)
+					);
+				}
 			}
 		}
 
@@ -92,13 +99,14 @@ fn do_json_test<H: FnMut(&str, HookType)>(json_data: &[u8], start_stop_hook: &mu
 	failed
 }
 
-declare_test!{TransactionTests_ttEip155VitaliksHomesead, "TransactionTests/ttEip155VitaliksHomesead"}
-declare_test!{TransactionTests_ttEip155VitaliksEip158, "TransactionTests/ttEip155VitaliksEip158"}
-declare_test!{TransactionTests_ttEip158, "TransactionTests/ttEip158"}
-declare_test!{TransactionTests_ttFrontier, "TransactionTests/ttFrontier"}
-declare_test!{TransactionTests_ttHomestead, "TransactionTests/ttHomestead"}
-declare_test!{TransactionTests_ttVRuleEip158, "TransactionTests/ttVRuleEip158"}
-declare_test!{TransactionTests_ttWrongRLPFrontier, "TransactionTests/ttWrongRLPFrontier"}
-declare_test!{TransactionTests_ttWrongRLPHomestead, "TransactionTests/ttWrongRLPHomestead"}
-declare_test!{TransactionTests_ttConstantinople, "TransactionTests/ttConstantinople"}
-declare_test!{TransactionTests_ttSpecConstantinople, "TransactionTests/ttSpecConstantinople"}
+declare_test!{TransactionTests_ttAddress, "TransactionTests/ttAddress"}
+declare_test!{TransactionTests_ttData, "TransactionTests/ttData"}
+declare_test!{TransactionTests_ttGasLimit, "TransactionTests/ttGasLimit"}
+declare_test!{TransactionTests_ttGasPrice, "TransactionTests/ttGasPrice"}
+declare_test!{TransactionTests_ttNonce, "TransactionTests/ttNonce"}
+declare_test!{TransactionTests_ttRSValue, "TransactionTests/ttRSValue"}
+declare_test!{TransactionTests_ttSignature, "TransactionTests/ttSignature"}
+declare_test!{TransactionTests_ttValue, "TransactionTests/ttValue"}
+declare_test!{TransactionTests_ttVValue, "TransactionTests/ttVValue"}
+declare_test!{TransactionTests_ttWrongRLP, "TransactionTests/ttWrongRLP"}
+

ethcore/transaction/src/transaction.rs

@@ -387,23 +387,6 @@ impl UnverifiedTransaction {
 		Ok(recover(&self.signature(), &self.unsigned.hash(self.chain_id()))?)
 	}
 
-	/// Do basic validation, checking for valid signature and minimum gas,
-	// TODO: consider use in block validation.
-	#[cfg(feature = "json-tests")]
-	pub fn validate(self, schedule: &Schedule, require_low: bool, allow_chain_id_of_one: bool, allow_empty_signature: bool)
-		-> Result<UnverifiedTransaction, error::Error>
-	{
-		let chain_id = if allow_chain_id_of_one { Some(1) } else { None };
-		self.verify_basic(require_low, chain_id, allow_empty_signature)?;
-		if !allow_empty_signature || !self.is_unsigned() {
-			self.recover_public()?;
-		}
-		if self.gas < U256::from(self.gas_required(&schedule)) {
-			return Err(error::Error::InvalidGasLimit(::unexpected::OutOfBounds{min: Some(U256::from(self.gas_required(&schedule))), max: None, found: self.gas}).into())
-		}
-		Ok(self)
-	}
-
 	/// Verify basic signature params. Does not attempt sender recovery.
 	pub fn verify_basic(&self, check_low_s: bool, chain_id: Option<u64>, allow_empty_signature: bool) -> Result<(), error::Error> {
 		if check_low_s && !(allow_empty_signature && self.is_unsigned()) {

evmbin/src/info.rs

@@ -84,7 +84,7 @@ pub fn run_action<T: Informant>(
 pub fn run_transaction<T: Informant>(
 	name: &str,
 	idx: usize,
-	spec: &ethjson::state::test::ForkSpec,
+	spec: &ethjson::spec::ForkSpec,
 	pre_state: &pod_state::PodState,
 	post_root: H256,
 	env_info: &client::EnvInfo,

json/src/blockchain/blockchain.rs

@@ -21,8 +21,7 @@ use hash::H256;
 use blockchain::state::State;
 use blockchain::header::Header;
 use blockchain::block::Block;
-use state::test::ForkSpec;
-use spec::{Genesis, Seal, Ethereum};
+use spec::{ForkSpec, Genesis, Seal, Ethereum};
 
 /// Blockchain deserialization.
 #[derive(Debug, PartialEq, Deserialize)]

json/src/spec/mod.rs

@@ -36,7 +36,7 @@ pub use self::account::Account;
 pub use self::builtin::{Builtin, Pricing, Linear};
 pub use self::genesis::Genesis;
 pub use self::params::Params;
-pub use self::spec::Spec;
+pub use self::spec::{Spec, ForkSpec};
 pub use self::seal::{Seal, Ethereum, AuthorityRoundSeal, TendermintSeal};
 pub use self::engine::Engine;
 pub use self::state::State;