Implement NoProof for json tests and update tests reference (replaces #9744) by cheme · Pull Request #9814 · openethereum/parity-ethereum

cheme · 2018-10-26T13:27:14Z

Closes #9590 .

Update to latest ethereum/tests .
Fix cost for constantinople test json.
Implements 'NoProof' engine from ethereum/tests#464 .

At this point with this PR everything pass except the case spotted by @holiman this week in https://gist.github.com/holiman/0154f00d5fcec5f89e85894cbb46fcb2 .

I skip those tests. For completeness, @sorpaas indicated that it is not an easy fix : so since those case cannot happen on a mainchain I did not open and reference an issue for them (in the json defining tests to skip I only added a comment but not issue reference).

Note for reviewer:

an other version of this PR exists with previous review comments is Implement NoProof for json tests and update tests reference #9744 , this PR replaces it .
when running NoProof with CanonNoSeal, the upper gas limit verification is required : I switch it from verify_block_basic to the header check. I got to put another trait method for it, a lighter change would be to make this check for every engine but it would change a bit the existing behavior (might not be an issue).

Block test are really not working so I disabled a few by commenting directly in source.

Since tests has been regenerated those one were failing on block difficulty check. Update ethereum/tests, waiting for cost fix (block test are still commented).

identified case). Fix block reward of constantinople json.

additional tests code.

verification, the test is running in `verify_header_param`. Note that test was previously only for ethash, and now for any engine.

cost of an additional trait method.

Update tests submodule to latest master (lot of new sstore tests passing)

synch).

parity-cla-bot · 2018-10-26T13:27:20Z

It looks like @cheme signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

niklasad1 · 2018-10-29T10:52:58Z


 	fn maximum_uncle_count(&self, _block: BlockNumber) -> usize { 2 }

+	fn maximum_gas_limit(&self) -> Option<U256> { Some(0x7fffffffffffffffu64.into()) }


Can you use underscores for this number to make it a little bit more readable?

niklasad1 · 2018-10-29T11:01:46Z

 	}
+	if let Some(limit) = engine.maximum_gas_limit() {
+		if header.gas_limit() > &limit {
+			return Err(From::from(::error::BlockError::InvalidGasLimit(OutOfBounds { min: None, max: Some(limit), found: header.gas_limit().clone() })));


clone() on copy-type

Suggested change

return Err(From::from(::error::BlockError::InvalidGasLimit(OutOfBounds { min: None, max: Some(limit), found: header.gas_limit().clone() })));

return Err(From::from(::error::BlockError::InvalidGasLimit(OutOfBounds { min: None, max: Some(limit), found: *header.gas_limit() })));

types.

niklasad1

Looks good, but I have too little knowledge of this to approve

andresilva

LGTM 👍 Just some whitespace grumbles.

andresilva · 2018-10-31T12:53:51Z

+		} else {
+			Self::from_spec(make_spec(chain))
+		};
+


stray newline

andresilva · 2018-10-31T12:54:02Z

+	}
+
+	fn from_spec_conf(spec: Spec, config: ClientConfig) -> Self {
+


stray newline

andresilva · 2018-10-31T12:55:56Z

 				}
 			}
 		}
+


stray newline. Actually would be nice if you could run a pretty printer on this file :)

…9744) (#9814) * Update test reference. Block test are really not working so I disabled a few by commenting directly in source. * Move ethtest commit cursor. * Implements 'NoProof' engine from ethereum/tests#464 . Since tests has been regenerated those one were failing on block difficulty check. Update ethereum/tests, waiting for cost fix (block test are still commented). * Update tests submodule reference to latest (all test passing except an identified case). Fix block reward of constantinople json. * Restore broken test by using old json tests files. * Use CanonNoSeal instead of a custom engine, still have to include some additional tests code. * Gas upper limit check in json_chain test was bad, moving the test to verification, the test is running in `verify_header_param`. Note that test was previously only for ethash, and now for any engine. * Restore old behavior (gas uper limit only for ethash engine), at the cost of an additional trait method. * Proper rpc test fix. * Update tests submodule, add SStore bug tests. * Fix json issue tabulation. Update tests submodule to latest master (lot of new sstore tests passing) * Switch ethereum/tests to tag 6.0.0-beta.1 (no tests changes from latest synch). * Display hex with separator, use indirection instead of clone for copy types.

* version: mark 2.2.0 beta * ci: remove failing tests for android, windows, and macos (#9788) * ci: remove failing tests for android, windows, and macos * ci: restore android build jobs * Move state root verification before gas used (#9841) * Classic.json Bootnode Update (#9828) * fix: Update bootnodes list to only responsive nodes * feat: Add more bootnodes to classic.json list * feat: Add retested bootnodes * Implement NoProof for json tests and update tests reference (replaces #9744) (#9814) * Update test reference. Block test are really not working so I disabled a few by commenting directly in source. * Move ethtest commit cursor. * Implements 'NoProof' engine from ethereum/tests#464 . Since tests has been regenerated those one were failing on block difficulty check. Update ethereum/tests, waiting for cost fix (block test are still commented). * Update tests submodule reference to latest (all test passing except an identified case). Fix block reward of constantinople json. * Restore broken test by using old json tests files. * Use CanonNoSeal instead of a custom engine, still have to include some additional tests code. * Gas upper limit check in json_chain test was bad, moving the test to verification, the test is running in `verify_header_param`. Note that test was previously only for ethash, and now for any engine. * Restore old behavior (gas uper limit only for ethash engine), at the cost of an additional trait method. * Proper rpc test fix. * Update tests submodule, add SStore bug tests. * Fix json issue tabulation. Update tests submodule to latest master (lot of new sstore tests passing) * Switch ethereum/tests to tag 6.0.0-beta.1 (no tests changes from latest synch). * Display hex with separator, use indirection instead of clone for copy types.

* version: mark 2.2.0 beta * ci: remove failing tests for android, windows, and macos (openethereum#9788) * ci: remove failing tests for android, windows, and macos * ci: restore android build jobs * Move state root verification before gas used (openethereum#9841) * Classic.json Bootnode Update (openethereum#9828) * fix: Update bootnodes list to only responsive nodes * feat: Add more bootnodes to classic.json list * feat: Add retested bootnodes * Implement NoProof for json tests and update tests reference (replaces openethereum#9744) (openethereum#9814) * Update test reference. Block test are really not working so I disabled a few by commenting directly in source. * Move ethtest commit cursor. * Implements 'NoProof' engine from ethereum/tests#464 . Since tests has been regenerated those one were failing on block difficulty check. Update ethereum/tests, waiting for cost fix (block test are still commented). * Update tests submodule reference to latest (all test passing except an identified case). Fix block reward of constantinople json. * Restore broken test by using old json tests files. * Use CanonNoSeal instead of a custom engine, still have to include some additional tests code. * Gas upper limit check in json_chain test was bad, moving the test to verification, the test is running in `verify_header_param`. Note that test was previously only for ethash, and now for any engine. * Restore old behavior (gas uper limit only for ethash engine), at the cost of an additional trait method. * Proper rpc test fix. * Update tests submodule, add SStore bug tests. * Fix json issue tabulation. Update tests submodule to latest master (lot of new sstore tests passing) * Switch ethereum/tests to tag 6.0.0-beta.1 (no tests changes from latest synch). * Display hex with separator, use indirection instead of clone for copy types.

cheme added 15 commits October 8, 2018 18:24

Update test reference.

4f6c6ac

Block test are really not working so I disabled a few by commenting directly in source.

Move ethtest commit cursor.

92b33a2

Merge branch 'master' into ethereum_tests_update

b503c56

Implements 'NoProof' engine from ethereum/tests#464 .

2ebbf41

Since tests has been regenerated those one were failing on block difficulty check. Update ethereum/tests, waiting for cost fix (block test are still commented).

Update tests submodule reference to latest (all test passing except an

a1eb0f2

identified case). Fix block reward of constantinople json.

Restore broken test by using old json tests files.

2275437

Use CanonNoSeal instead of a custom engine, still have to include some

cef5d5a

additional tests code.

Gas upper limit check in json_chain test was bad, moving the test to

a6eecf6

verification, the test is running in `verify_header_param`. Note that test was previously only for ethash, and now for any engine.

Restore old behavior (gas uper limit only for ethash engine), at the

a2e170e

cost of an additional trait method.

Proper rpc test fix.

0683616

Update tests submodule, add SStore bug tests.

fdf2baf

Merge branch 'master' into ethereum_tests_update

5964c72

Fix json issue tabulation.

b82d0b0

Update tests submodule to latest master (lot of new sstore tests passing)

Switch ethereum/tests to tag 6.0.0-beta.1 (no tests changes from latest

55acd8b

synch).

Merge branch 'master' into ethereum-tests-2

8b29e0e

cheme added A0-pleasereview 🤓 Pull request needs code review. B1-patch-beta 🕷🕷 M4-core ⛓ Core client code / Rust. B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. labels Oct 26, 2018

cheme requested a review from sorpaas October 26, 2018 13:27

cheme mentioned this pull request Oct 26, 2018

Implement NoProof for json tests and update tests reference #9744

Closed

sorpaas approved these changes Oct 26, 2018

View reviewed changes

5chdn mentioned this pull request Oct 27, 2018

Backports: parity beta 2.1.4 #9787

Merged

13 tasks

5chdn added this to the 2.3 milestone Oct 29, 2018

5chdn added the B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. label Oct 29, 2018

niklasad1 reviewed Oct 29, 2018

View reviewed changes

Display hex with separator, use indirection instead of clone for copy

2405f93

types.

niklasad1 reviewed Oct 29, 2018

View reviewed changes

andresilva approved these changes Oct 31, 2018

View reviewed changes

sorpaas mentioned this pull request Oct 31, 2018

Move state root verification before gas used #9841

Merged

Merge branch 'master' into ethereum-test-update2

c424a77

5chdn merged commit a511264 into openethereum:master Nov 1, 2018

This was referenced Nov 1, 2018

version: mark 2.2.0 beta #9820

Merged

version: mark 2.1.5 stable #9821

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement NoProof for json tests and update tests reference (replaces #9744)#9814

Implement NoProof for json tests and update tests reference (replaces #9744)#9814
5chdn merged 17 commits into
openethereum:masterfrom
cheme:ethereum-test-update2

cheme commented Oct 26, 2018

Uh oh!

parity-cla-bot commented Oct 26, 2018

Uh oh!

niklasad1 Oct 29, 2018

Uh oh!

niklasad1 Oct 29, 2018

Uh oh!

niklasad1 left a comment

Uh oh!

andresilva left a comment

Uh oh!

andresilva Oct 31, 2018

Uh oh!

andresilva Oct 31, 2018

Uh oh!

andresilva Oct 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants


		fn maximum_uncle_count(&self, _block: BlockNumber) -> usize { 2 }

		fn maximum_gas_limit(&self) -> Option<U256> { Some(0x7fffffffffffffffu64.into()) }

	return Err(From::from(::error::BlockError::InvalidGasLimit(OutOfBounds { min: None, max: Some(limit), found: header.gas_limit().clone() })));
	return Err(From::from(::error::BlockError::InvalidGasLimit(OutOfBounds { min: None, max: Some(limit), found: *header.gas_limit() })));

		}

		fn from_spec_conf(spec: Spec, config: ClientConfig) -> Self {

Conversation

cheme commented Oct 26, 2018

Uh oh!

parity-cla-bot commented Oct 26, 2018

Uh oh!

niklasad1 Oct 29, 2018

Choose a reason for hiding this comment

Uh oh!

niklasad1 Oct 29, 2018

Choose a reason for hiding this comment

Uh oh!

niklasad1 left a comment

Choose a reason for hiding this comment

Uh oh!

andresilva left a comment

Choose a reason for hiding this comment

Uh oh!

andresilva Oct 31, 2018

Choose a reason for hiding this comment

Uh oh!

andresilva Oct 31, 2018

Choose a reason for hiding this comment

Uh oh!

andresilva Oct 31, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants