Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

Add constantinople conf to EvmTestClient. #9570

Merged
5chdn merged 14 commits into from
Sep 25, 2018
Merged

Add constantinople conf to EvmTestClient. #9570

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
82d46b2
Add constantinople conf to EvmTestClient.
cheme Sep 17, 2018
ef2372d
Skip some test to update submodule etheureum/tests submodule to latest.
cheme Sep 17, 2018
371a1a6
Put skipping 'under issue' test behind a feature.
cheme Sep 17, 2018
b61ec3f
Change blockReward for const-test to pass ethereum/tests
sorpaas Sep 17, 2018
cdf2435
Merge branch 'master' into constantinople_evmtest
cheme Sep 18, 2018
5c39eb2
Update tests to new constantinple definition (change of reward at block
cheme Sep 18, 2018
6686db7
Fix modexp and bn128_mul gas prices in chain config
sorpaas Sep 18, 2018
e8a30bb
Changes `run_test_path` method to append its directory results (without
cheme Sep 18, 2018
545654e
retab current.json
cheme Sep 18, 2018
48f23b4
Update reference to parity issue for failing tests.
cheme Sep 19, 2018
01e0398
Merge branch 'master' into constantinople_evmtest
cheme Sep 19, 2018
292b05c
Merge branch 'master' into constantinople_evmtest
cheme Sep 19, 2018
68da3e8
Merge branch 'master' into constantinople_evmtest
cheme Sep 24, 2018
1328aa1
Merge branch 'master' into constantinople_evmtest
cheme Sep 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ethcore/res/ethereum/tests
Open in desktop
Submodule tests updated 369 files
215 changes: 215 additions & 0 deletions ethcore/res/ethereum/tests-issues/currents.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
{ "block":
Copy link
Collaborator

@sorpaas sorpaas Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this file to use tabs for indentation.

[

{
"reference": 9568,
Copy link
Collaborator

@sorpaas sorpaas Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this type to a string? Sometimes it may be easier to reference it via ethereum/tests's issue number.

"failing": "stCreateTest",
"subtests": [
"CreateOOGafterInitCodeRevert_d0g0v0_Constantinople",
"CreateOOGafterInitCodeReturndata3_d0g0v0_Constantinople",
"CreateOOGafterInitCodeRevert2_d0g0v0_Constantinople",
"CreateOOGafterInitCode_d0g1v0_Constantinople",
"CreateOOGafterInitCodeReturndataSize_d0g0v0_Constantinople",
"CreateOOGafterInitCode_d0g0v0_Constantinople",
"CreateOOGafterInitCodeReturndata2_d0g0v0_Constantinople",
Copy link
Collaborator

@sorpaas sorpaas Sep 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please cherry-pick 544f6a3
I think the only one left here right now should be CreateOOGafterInitCodeReturndata2.

"CreateOOGafterInitCodeReturndata_d0g1v0_Constantinople",
"CreateOOGafterInitCodeReturndata2_d0g1v0_Constantinople",
"CreateOOGafterInitCodeReturndata_d0g0v0_Constantinople"
]
},
{
"reference": 9568,
"failing": "stEIP158Specific",
"subtests": [
"callToEmptyThenCallError_d0g0v0_Byzantium",
"callToEmptyThenCallError_d0g0v0_Constantinople",
"callToEmptyThenCallError_d0g0v0_EIP158"
]
},
{
"reference": 9568,
"failing": "stPreCompiledContracts",
"subtests": [
"identity_to_smaller_d0g0v0_Constantinople",
"identity_to_bigger_d0g0v0_Constantinople"
]
},
{
"reference": 9568,
"failing": "stReturnDataTest",
"subtests": [
"modexp_modsize0_returndatasize_d0g1v0_Constantinople",
"modexp_modsize0_returndatasize_d0g2v0_Constantinople",
"modexp_modsize0_returndatasize_d0g3v0_Constantinople"
]
},
{
"reference": 9568,
"failing": "stRevertTest",
"subtests": [
"RevertOnEmptyStack_d0g0v0_Constantinople"
Copy link
Collaborator

@sorpaas sorpaas Sep 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is fixed? Right now what I get from GeneralStateTest_stRevertTest is only ["LoopCallsDepthThenRevert2"].

]
},
{
"reference": 9568,
"failing": "stSpecialTest",
"subtests": [
"FailedCreateRevertsDeletion_d0g0v0_Constantinople",
Copy link
Collaborator

@sorpaas sorpaas Sep 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one doesn't appear any more with 544f6a3

"push32withoutByte_d0g0v0_Constantinople"
]
},
{
"reference": 9568,
"failing": "stSystemOperationsTest",
Copy link
Collaborator

@sorpaas sorpaas Sep 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed as well with 544f6a3

"subtests": [
"doubleSelfdestructTest_d0g0v0_Constantinople",
"doubleSelfdestructTest2_d0g0v0_Constantinople"
]
}


],
"state":
[
{
"reference": 9568,
"failing": "stCreateTest",
"subtests": {
"CreateOOGafterInitCodeReturndata2": {
"subnumbers": ["2"],
"chain": "Constantinople (test)"
}
}
},
{
"reference": 9568,
"failing": "stEIP150Specific",
"subtests": {
"NewGasPriceForCodes": {
"subnumbers": ["1"],
"chain": "Constantinople (test)"
}
}
},
{
"reference": 9568,
"failing": "stInitCodeTest",
"subtests": {
"OutOfGasContractCreation": {
"subnumbers": ["4"],
"chain": "Constantinople (test)"
}
}
},
{
"reference": 9568,
"failing": "stPreCompiledContracts",
"subtests": {
"modexp": {
"subnumbers": ["*"],
"chain": "Constantinople (test)"
}
}
},
{
"reference": 9568,
"failing": "stRevertTest",
"subtests": {
"LoopCallsDepthThenRevert3": {
"subnumbers": ["1"],
"chain": "Constantinople (test)"
},
"RevertOpcodeCreate": {
"subnumbers": ["1"],
"chain": "Constantinople (test)"
},
"RevertSubCallStorageOOG2": {
"subnumbers": ["1","3"],
"chain": "Constantinople (test)"
},
"RevertDepthCreateOOG": {
"subnumbers": ["3","4"],
"chain": "Constantinople (test)"
},
"RevertOpcodeMultipleSubCalls": {
"subnumbers": ["*"],
"chain": "Constantinople (test)"
},
"RevertOpcodeDirectCall": {
"subnumbers": ["1","2"],
"chain": "Constantinople (test)"
},
"LoopCallsDepthThenRevert2": {
"subnumbers": ["1"],
"chain": "Constantinople (test)"
},
"RevertDepth2": {
"subnumbers": ["1"],
"chain": "Constantinople (test)"
},
"RevertRemoteSubCallStorageOOG2": {
"subnumbers": ["1","2"],
"chain": "Constantinople (test)"
},
"RevertDepthCreateAddressCollision": {
"subnumbers": ["3","4"],
"chain": "Constantinople (test)"
}
}
},
{
"reference": 9568,
"failing": "stStaticCall",
Copy link
Collaborator

@sorpaas sorpaas Sep 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are passing except static_checkOpcodes2.

"subtests": {
"static_callCreate": {
"subnumbers": ["2"],
"chain": "Constantinople (test)"
},
"static_CheckOpcodes4": {
"subnumbers": ["3"],
"chain": "Constantinople (test)"
},
"static_CheckOpcodes3": {
"subnumbers": ["5","6","7","8"],
"chain": "Constantinople (test)"
},
"static_RevertDepth2": {
"subnumbers": ["1"],
"chain": "Constantinople (test)"
},
"static_callBasic": {
"subnumbers": ["1","2"],
"chain": "Constantinople (test)"
},
"static_CheckOpcodes2": {
"subnumbers": ["5","6","7","8"],
"chain": "Constantinople (test)"
}
}
},
{
"reference": 9568,
"failing": "stZeroKnowledge",
"subtests": {
"pointAddTrunc": {
Copy link
Collaborator

@sorpaas sorpaas Sep 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like those are passing except pointAdd!

Copy link
Contributor Author

@cheme cheme Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is something not so good with the way the test are currently running : having a first failure is hiding other tests failure (it reports only one failure but if I skip it, it shows the other ones). So the other test in this branch are still failing for me.

Copy link
Collaborator

@sorpaas sorpaas Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those one are all actually passing except pointAdd? @cheme Do you mind to check this?

Copy link
Contributor Author

@cheme cheme Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not passing :( , there is something bad with our current code : if you run the test without skipping anything it fails on only one test, but as soon as you skip it, it reveals others failure.

Copy link
Collaborator

@sorpaas sorpaas Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it! Can you also cherry-pick 16f661a? Our previous definition of modexp and bn128_mul has wrong gas price. I think it may fix some other tests.

Copy link
Contributor Author

@cheme cheme Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it does reduce the number of failing zk tests. Many still don't pass, but it is due to ethereum/tests#512 (with 1283 disabled everything is fine).

"subnumbers": ["*"],
"chain": "Constantinople (test)"
},
"pointAdd": {
Copy link
Collaborator

@sorpaas sorpaas Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is indeed ethereum/tests's issue: ethereum/tests#512

"subnumbers": ["*"],
"chain": "Constantinople (test)"
},
"pointMulAdd": {
"subnumbers": ["*"],
"chain": "Constantinople (test)"
},
"pointMulAdd2": {
"subnumbers": ["*"],
"chain": "Constantinople (test)"
}

}
}

]
}
2 changes: 1 addition & 1 deletion ethcore/src/client/evm_test_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ impl<'a> EvmTestClient<'a> {
ForkSpec::EIP150 => Some(ethereum::new_eip150_test()),
ForkSpec::EIP158 => Some(ethereum::new_eip161_test()),
ForkSpec::Byzantium => Some(ethereum::new_byzantium_test()),
ForkSpec::Constantinople => Some(ethereum::new_constantinople_test()),
ForkSpec::EIP158ToByzantiumAt5 => Some(ethereum::new_transition_test()),
ForkSpec::FrontierToHomesteadAt5 | ForkSpec::HomesteadToDaoAt5 | ForkSpec::HomesteadToEIP150At5 => None,
_ => None,
}
}

Expand Down
9 changes: 9 additions & 0 deletions ethcore/src/json_tests/chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use miner::Miner;
use io::IoChannel;
use test_helpers;
use verification::queue::kind::blocks::Unverified;
use super::SKIP_TEST_STATE;

use super::HookType;

Expand All @@ -36,12 +37,20 @@ pub fn run_test_file<H: FnMut(&str, HookType)>(p: &Path, h: &mut H) {
::json_tests::test_common::run_test_file(p, json_chain_test, h)
}

fn skip_test(name: &String) -> bool {
SKIP_TEST_STATE.block.iter().any(|block_test|block_test.subtests.contains(name))
}

pub fn json_chain_test<H: FnMut(&str, HookType)>(json_data: &[u8], start_stop_hook: &mut H) -> Vec<String> {
::ethcore_logger::init_log();
let tests = ethjson::blockchain::Test::load(json_data).unwrap();
let mut failed = Vec::new();

for (name, blockchain) in tests.into_iter() {
if skip_test(&name) {
println!(" - {} | {:?} Ignoring tests because in skip list", name, blockchain.network);
continue;
}
start_stop_hook(&name, HookType::OnStart);

let mut fail = false;
Expand Down
3 changes: 3 additions & 0 deletions ethcore/src/json_tests/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ mod executive;
mod state;
mod chain;
mod trie;
mod skip;

#[cfg(test)]
mod difficulty;


pub use self::test_common::HookType;

pub use self::transaction::run_test_path as run_transaction_test_path;
Expand All @@ -42,3 +44,4 @@ pub use self::trie::run_generic_test_path as run_generic_trie_test_path;
pub use self::trie::run_generic_test_file as run_generic_trie_test_file;
pub use self::trie::run_secure_test_path as run_secure_trie_test_path;
pub use self::trie::run_secure_test_file as run_secure_trie_test_file;
use self::skip::SKIP_TEST_STATE;
29 changes: 29 additions & 0 deletions ethcore/src/json_tests/skip.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2015-2018 Parity Technologies (UK) Ltd.
// This file is part of Parity.

// Parity is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Parity is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Parity. If not, see <http://www.gnu.org/licenses/>.

//! State tests to skip.

use ethjson;

lazy_static!{


pub static ref SKIP_TEST_STATE: ethjson::test::SkipStates = {
let skip_data = include_bytes!("../../res/ethereum/tests-issues/currents.json");
ethjson::test::SkipStates::load(&skip_data[..]).expect("No invalid json allowed")
};

}
18 changes: 17 additions & 1 deletion ethcore/src/json_tests/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use client::{EvmTestClient, EvmTestError, TransactResult};
use ethjson;
use transaction::SignedTransaction;
use vm::EnvInfo;

use super::SKIP_TEST_STATE;
use super::HookType;

/// Run state jsontests on a given folder.
Expand All @@ -35,6 +35,18 @@ pub fn run_test_file<H: FnMut(&str, HookType)>(p: &Path, h: &mut H) {
::json_tests::test_common::run_test_file(p, json_chain_test, h)
}

fn skip_test(subname: &str, chain: &String, number: usize) -> bool {
SKIP_TEST_STATE.state.iter().any(|state_test|{
if let Some(subtest) = state_test.subtests.get(subname) {
chain == &subtest.chain &&
(subtest.subnumbers[0] == "*"
|| subtest.subnumbers.contains(&number.to_string()))
} else {
false
}
})
}

pub fn json_chain_test<H: FnMut(&str, HookType)>(json_data: &[u8], start_stop_hook: &mut H) -> Vec<String> {
::ethcore_logger::init_log();
let tests = ethjson::state::test::Test::load(json_data).unwrap();
Expand All @@ -60,6 +72,10 @@ pub fn json_chain_test<H: FnMut(&str, HookType)>(json_data: &[u8], start_stop_ho

for (i, state) in states.into_iter().enumerate() {
let info = format!(" - {} | {:?} ({}/{}) ...", name, spec_name, i + 1, total);
if skip_test(&name, &spec.name, i + 1) {
println!("{} in skip list : SKIPPED", info);
continue;
}

let post_root: H256 = state.hash.into();
let transaction: SignedTransaction = multitransaction.select(&state.indexes).into();
Expand Down
Loading