Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove a test case from badOpcodes #592

Merged
merged 1 commit into from
Apr 24, 2019
Merged

remove a test case from badOpcodes #592

merged 1 commit into from
Apr 24, 2019

Conversation

jwasinger
Copy link
Contributor

Was mistakenly including PC in the badOpcodes test.

@@ -95,11 +96,12 @@
},
"badOpcodes_d101g0v0_EIP158" : {
"_info" : {
"binaryenVersion" : "BINARYEN_VERSION",
"comment" : "",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wtf is binaryen? @axic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a wasm interpreter. Only relevant for stEwasmTests but somehow made it into this repository by accident.

@winsvega
Copy link
Collaborator

@jwasinger please refill the tests on latest aleth. (without Binaryen)

@jwasinger
Copy link
Contributor Author

This was filled with the latest aleth.

@jwasinger
Copy link
Contributor Author

Oh I see. This binaryen issue was solved. I'll refill the tests.

@@ -37,7 +24,7 @@
},
{
"indexes" : {
"data" : [115],
"data" : [114],
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is the expect section for all other data for network >=Constantinople ?
if there is no expect section, then there will be no test generated.
(for example there will be no test for data 100 and network Constantinople)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right.

Copy link
Collaborator

@winsvega winsvega left a comment

Choose a reason for hiding this comment

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

where is the expect section for all other data for network >=Constantinople ?
if there is no expect section, then there will be no test generated.
(for example there will be no test for data 100 and network Constantinople)

@jwasinger jwasinger force-pushed the badOpcodesFix branch 2 times, most recently from 63d300a to 129828f Compare March 27, 2019 21:22
@jwasinger
Copy link
Contributor Author

I force pushed to only include the state test filler until I figure out why test cases are not passing:

Test Case "stBadOpcode":
100%
/home/ewasm/projects/aleth/test/tools/libtesteth/ImportTest.cpp(565): error: in "GeneralStateTests/stBadOpcode": badOpcodes_d120g0v0_Homestead Compare States: 6295ee1b4f6dd65047762f924ecd367c17eabf8f address not expected to exist!
Constantinople data: 4 gas: 0 val: 0
/home/ewasm/projects/aleth/test/tools/libtesteth/ImportTest.cpp(565): error: in "GeneralStateTests/stBadOpcode": badOpcodes_d120g0v0_Homestead Compare States: 6295ee1b4f6dd65047762f924ecd367c17eabf8f address not expected to exist!
Constantinople data: 5 gas: 0 val: 0
/home/ewasm/projects/aleth/test/tools/libtesteth/ImportTest.cpp(565): error: in "GeneralStateTests/stBadOpcode": badOpcodes_d120g0v0_Homestead Compare States: 6295ee1b4f6dd65047762f924ecd367c17eabf8f address not expected to exist!
Constantinople data: 6 gas: 0 val: 0
/home/ewasm/projects/aleth/test/tools/libtesteth/ImportTest.cpp(565): error: in "GeneralStateTests/stBadOpcode": badOpcodes_d120g0v0_Homestead Compare States: 6295ee1b4f6dd65047762f924ecd367c17eabf8f address not expected to exist!
Constantinople data: 24 gas: 0 val: 0
/home/ewasm/projects/aleth/test/tools/libtesteth/ImportTest.cpp(565): error: in "GeneralStateTests/stBadOpcode": badOpcodes_d120g0v0_Homestead Compare States: 6295ee1b4f6dd65047762f924ecd367c17eabf8f address not expected to exist!
ConstantinopleFix data: 4 gas: 0 val: 0
/home/ewasm/projects/aleth/test/tools/libtesteth/ImportTest.cpp(565): error: in "GeneralStateTests/stBadOpcode": badOpcodes_d120g0v0_Homestead Compare States: 6295ee1b4f6dd65047762f924ecd367c17eabf8f address not expected to exist!
ConstantinopleFix data: 5 gas: 0 val: 0
/home/ewasm/projects/aleth/test/tools/libtesteth/ImportTest.cpp(565): error: in "GeneralStateTests/stBadOpcode": badOpcodes_d120g0v0_Homestead Compare States: 6295ee1b4f6dd65047762f924ecd367c17eabf8f address not expected to exist!
ConstantinopleFix data: 6 gas: 0 val: 0
/home/ewasm/projects/aleth/test/tools/libtesteth/ImportTest.cpp(565): error: in "GeneralStateTests/stBadOpcode": badOpcodes_d120g0v0_Homestead Compare States: 6295ee1b4f6dd65047762f924ecd367c17eabf8f address not expected to exist!
ConstantinopleFix data: 24 gas: 0 val: 0

@jwasinger
Copy link
Contributor Author

test case 4:

[1] PUSH1 0x01
[3] PUSH1 0x01
[5] PUSH1 0x01
[7] PUSH1 0x01
[9] PUSH1 0x01
[11] PUSH1 0x01
[13] PUSH1 0x01
[15] PUSH1 0x01
[16] '1b'(Unknown Opcode)
[18] PUSH1 0x00
[20] PUSH1 0x00
[21] RETURN

test case 5:

[1] PUSH1 0x01
[3] PUSH1 0x01
[5] PUSH1 0x01
[7] PUSH1 0x01
[9] PUSH1 0x01
[11] PUSH1 0x01
[13] PUSH1 0x01
[15] PUSH1 0x01
[16] '1c'(Unknown Opcode)
[18] PUSH1 0x00
[20] PUSH1 0x00
[21] RETURN

Is aleth using these opcodes for prototyping?

@winsvega
Copy link
Collaborator

@chfast, is aleth recognize this opcodes as valid when it should not ?

@jwasinger
Copy link
Contributor Author

Okay, these are bit shifting opcodes.

@jwasinger
Copy link
Contributor Author

And another one that is being updated (removed from the list of invalid opcodes) is EXTCODEHASH.

@winsvega
Copy link
Collaborator

winsvega commented Apr 2, 2019

Ok

@winsvega winsvega merged commit 3d0724b into develop Apr 24, 2019
@jwasinger jwasinger deleted the badOpcodesFix branch April 24, 2019 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants