Update state tests execution model #9440

tomusdrw · 2018-08-30T17:25:39Z

Touches the coinbase at the end of the execution
Updates the state tests to a version that supports that
As a consequence updates the transactions tests to a new format

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

…des constantinople changes). Added test `json_tests::trie::generic::TrieTests_trieanyorder` and a few difficulty tests.

implement it.

some errors.

cheme

LGTM (I also push some difficulty test, and push submodule commit reference at the same as geth)

dvdplm · 2018-08-31T07:02:28Z

For reviewers it's important to note that you need to check out rev b8a21c193696976ca3b33b6d82107601063a5d26 in ethcore/res/ethereum/tests or bad things happen.

tomusdrw · 2018-08-31T07:05:51Z

@dvdplm it should be fine to just do git submodule update after git pull, since the correct submodule revision should be fetched automagically.

dvdplm

Looks good.

I'm curious about your subjective assessment on the quality/usefulness of these tests: do you think this will be helpful to achieve better stability or is it more of a correctness harness? Is it worth our time in your opinion?

dvdplm · 2018-08-31T07:03:30Z

ethcore/res/ethereum/eip161_test.json

@@ -24,7 +24,7 @@
 		"eip161dTransition": "0x0",
 		"eip98Transition": "0x7fffffffffffffff",
 		"eip86Transition": "0x7fffffffffffffff",
-		"eip155Transition": "0x7fffffffffffffff",
+		"eip155Transition": "0x0",


Setting to 0 enables the feature right?

Yes, otherwise some transaction tests for EIP158 spec (which for us is actaully eip161_test.json) fail, cause they assume that eip155 is enabled as well.

dvdplm · 2018-08-31T07:06:41Z

ethcore/src/json_tests/test_common.rs

@@ -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


Yeah, there are some swp files or .stub files committed in the repo :(

dvdplm · 2018-08-31T07:15:34Z

ethcore/src/json_tests/transaction.rs

+				.and_then(|t: UnverifiedTransaction| {
+					let mut header: Header = Default::default();
+					// Use high enough number to activate all required features.
+					header.set_number(0x6ffffffffffffe);


Perhaps make it this number a named const (for readability)? e.g. FEATURE_ENABLING_BLOCKNR or some such?

Makes sense.

dvdplm · 2018-08-31T07:27:48Z

should be fine to just do git submodule update

Wouldn't that update wasm-tests as well? That might be undesireable?

tomusdrw

Wouldn't that update wasm-tests as well

It should only update to a version specified in .gitmodules file, so it should be fine. If you want to change that version I believe it has to be done manually (cd and git checkout, then git commit)

I'm curious about your subjective assessment on the quality/usefulness of these tests: do you think this will be helpful to achieve better stability or is it more of a correctness harness?

IMHO the tests are far from being perfect, but they are the best that (afaict) we currently have. They are currently critical to give us at least minimal confidence that there are no consensus issues between various implementations. I'm in favour of supporting them (especially in the periods between upcoming hfs), but I'd also be really excited to see some new, better test cases (especially some that are easier to debug).

tomusdrw · 2018-08-31T07:43:28Z

ethcore/src/json_tests/test_common.rs

@@ -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


Yeah, there are some swp files or .stub files committed in the repo :(

tomusdrw · 2018-08-31T07:43:47Z

ethcore/src/json_tests/transaction.rs

+				.and_then(|t: UnverifiedTransaction| {
+					let mut header: Header = Default::default();
+					// Use high enough number to activate all required features.
+					header.set_number(0x6ffffffffffffe);


Makes sense.

andresilva

LGTM. Can we rebase/merge to fix conflict with eip161_test.json?

sorpaas · 2018-09-08T21:58:01Z

Please merge master. I think we may face the same test fail as py-evm after #9319, where it fixed the bug in discard_checkpoint if a new contract is created but the account already exists.

merged

* 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

* ethash: implement EIP-1234 (#9187) * Implement EIP-1052 (EXTCODEHASH) and fix several issues in state account cache (#9234) * Implement EIP-1052 and fix several issues related to account cache * Fix jsontests * Merge two matches together * Avoid making unnecessary Arc<Vec> * Address grumbles * Comply EIP-86 with the new definition (#9140) * Comply EIP-86 with the new CREATE2 opcode * Fix rpc compile * Fix interpreter CREATE/CREATE2 stack pop difference * Add unreachable! to fix compile * Fix instruction_info * Fix gas check due to new stack item * Add new tests in executive * Fix have_create2 comment * Remove all unused references of eip86_transition and block_number * Implement KIP4: create2 for wasm (#9277) * Basic implementation for kip4 * Add KIP-4 config flags * typo: docs fix * Fix args offset * Add tests for create2 * tests: evm * Update wasm-tests and fix all gas costs * Update wasm-tests * Update wasm-tests and fix gas costs * `gasleft` extern implemented for WASM runtime (kip-6) (#9357) * Wasm gasleft extern added * wasm_gasleft_activation_transition -> kip4_transition * use kip-6 switch * gasleft_panic -> gasleft_fail rename * call_msg_gasleft test added and gas_left agustments because this openethereum/wasm-tests#52 * change .. to _ * fix comment for the have_gasleft param * update tests (openethereum/wasm-tests@0edbf86) * Add EIP-1014 transition config flag (#9268) * Add EIP-1014 transition config flag * Remove EIP-86 configs * Change CREATE2 opcode index to 0xf5 * Move salt to the last item in the stack * Change sendersaltandaddress scheme to comply with current EIP-1014 * Fix json configs * Fix create2 test * Fix deprecated comments * EIP 1283: Net gas metering for SSTORE without dirty maps (#9319) * Implement last_checkpoint_storage_at * Add reverted_storage_at for externalities * sstore_clears_count -> sstore_clears_refund * Implement eip1283 for evm * Add eip1283Transition params * evm: fix tests * jsontests: fix test * Return checkpoint index when creating * Comply with spec Version II * Fix docs * Fix jsontests feature compile * Address grumbles * Fix no-checkpoint-entry case * Remove unnecessary expect * Add test for State::checkpoint_storage_at * Add executive level test for eip1283 * Hard-code transaction_checkpoint_index to 0 * Fix jsontests * Add tests for checkpoint discard/revert * Require checkpoint to be empty for kill_account and commit * Get code coverage * Use saturating_add/saturating_sub * Fix issues in insert_cache * Clear the state again * Fix original_storage_at * Early return for empty RLP trie storage * Update comments * Fix borrow_mut issue * Simplify checkpoint_storage_at if branches * Better commenting for gas handling code * Address naming grumbles * More tests * Fix an issue in overwrite_with * Add another test * Fix comment * Remove unnecessary bracket * Move orig to inner if * Remove test coverage for this PR * Add tests for executive original value * Add warn! for an unreachable cause * 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 * Fix checkpointing when creating contract failed (#9514) * In create memory calculation is the same for create2 because the additional parameter was popped before. (#9522) * Enable all Constantinople hard fork changes in constantinople_test.json (#9505) * Enable all Constantinople hard fork changes in constantinople_test.json * Address grumbles * Remove EIP-210 activation * 8m -> 5m * Temporarily add back eip210 transition so we can get test passed * Add eip210_test and remove eip210 transition from const_test * Add constantinople conf to EvmTestClient. (#9570) * Add constantinople conf to EvmTestClient. * Skip some test to update submodule etheureum/tests submodule to latest. * Put skipping 'under issue' test behind a feature. * Change blockReward for const-test to pass ethereum/tests * Update tests to new constantinple definition (change of reward at block 5). Switch 'reference' to string, that way we can include issues from others repo (more flexible)Update tests to new constantinple definition (change of reward at block 5). Switch 'reference' to string, that way we can include issues from others repo (more flexible). * Fix modexp and bn128_mul gas prices in chain config * Changes `run_test_path` method to append its directory results (without that it stop testing at the first file failure). Add some missing tests. Add skip for those (block create2 is one hundred percent false but on hive we can see that geth and aleth got similar issue for this item). * retab current.json * Update reference to parity issue for failing tests. * Hardfork the testnets (#9562) * ethcore: propose hardfork block number 4230000 for ropsten * ethcore: propose hardfork block number 9000000 for kovan * ethcore: enable kip-4 and kip-6 on kovan * etcore: bump kovan hardfork to block 9.2M * ethcore: fix ropsten constantinople block number to 4.2M * ethcore: disable difficulty_test_ropsten until ethereum/tests are updated upstream * Don't hash the init_code of CREATE. (#9688) * Implement CREATE2 gas changes and fix some potential overflowing (#9694) * Implement CREATE2 gas changes and fix some potential overflowing * Ignore create2 state tests * Split CREATE and CREATE2 in gasometer * Generalize rounding (x + 31) / 32 to to_word_size * ethcore: delay ropsten hardfork (#9704) * Add hardcoded headers (#9730) * add foundation hardcoded header #6486017 * add ropsten hardcoded headers #4202497 * add kovan hardcoded headers #9023489 * gitlab ci: releasable_branches: change variables condition to schedule (#9729) * HF in POA Core (2018-10-22) (#9724) poanetwork/poa-chain-spec#87

tomusdrw and others added 7 commits August 30, 2018 15:03

Update & fix JSON state tests.

1b20809

Update tests to be able to run ethtest at

bfb380e

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

Switch to same commit for submodule ethereum/test as geth (next inclu…

b8188c5

…des 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

83f48e6

implement it.

Support new (shitty) format of transaction tests.

5dbae4f

Ignore junk in ethereum/tests repo.

f6aa443

Ignore incorrect test.

1d0aecc

tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Aug 30, 2018

cheme added 2 commits August 30, 2018 19:57

Update to a later commit

8321952

Merge my branch into td-statetests, mostly keep Tomus version and fix

38dc09b

some errors.

5chdn added this to the 2.1 milestone Aug 30, 2018

cheme approved these changes Aug 31, 2018

View reviewed changes

dvdplm approved these changes Aug 31, 2018

View reviewed changes

tomusdrw commented Aug 31, 2018

View reviewed changes

tomusdrw added 2 commits August 31, 2018 10:20

Move block number to a constant.

25c6a42

Fix ZK2 test - touched account should also be cleared.

60dceb2

andresilva previously requested changes Sep 5, 2018

View reviewed changes

Merge branch 'master' into td-statetests

f68476d

cheme added 3 commits September 10, 2018 13:29

Merge branch 'master' into td-statetests

86ead07

Merge branch 'master' into td-statetests

712d6d1

Fix conflict resolution

e355c6d

5chdn added the A8-looksgood 🦄 Pull request is reviewed well. label Sep 10, 2018

5chdn merged commit 6e5a1c0 into master Sep 10, 2018

5chdn deleted the td-statetests branch September 10, 2018 20:38

This was referenced Sep 18, 2018

Add constantinople conf to EvmTestClient. #9570

Merged

Currently failing JSON tests #9590

Closed

andresilva mentioned this pull request Oct 9, 2018

[stable] Backport constantinople changes #9723

Merged

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update state tests execution model #9440

Update state tests execution model #9440

tomusdrw commented Aug 30, 2018

cheme left a comment

dvdplm commented Aug 31, 2018

tomusdrw commented Aug 31, 2018

dvdplm left a comment

dvdplm Aug 31, 2018

tomusdrw Aug 31, 2018

dvdplm Aug 31, 2018

tomusdrw Aug 31, 2018

dvdplm Aug 31, 2018

tomusdrw Aug 31, 2018

dvdplm commented Aug 31, 2018

tomusdrw left a comment

tomusdrw Aug 31, 2018

tomusdrw Aug 31, 2018

andresilva left a comment

sorpaas commented Sep 8, 2018

Update state tests execution model #9440

Update state tests execution model #9440

Conversation

tomusdrw commented Aug 30, 2018

cheme left a comment

Choose a reason for hiding this comment

dvdplm commented Aug 31, 2018

tomusdrw commented Aug 31, 2018

dvdplm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvdplm commented Aug 31, 2018

tomusdrw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andresilva left a comment

Choose a reason for hiding this comment

sorpaas commented Sep 8, 2018