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

[json-tests] stRevertTest blockchain test failures #11073

Closed
dvdplm opened this issue Sep 20, 2019 · 4 comments · Fixed by #11597
Closed

[json-tests] stRevertTest blockchain test failures #11073

dvdplm opened this issue Sep 20, 2019 · 4 comments · Fixed by #11597
Labels
F4-tests 💻 Tests need fixing, improving or augmenting. P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible.

Comments

@dvdplm
Copy link
Collaborator

dvdplm commented Sep 20, 2019

After #11054 the following JSON tests fail in stRevertTest and are added to the list of skipped tests in ethcore/res/ethereum/tests-issues/currents.json.

"RevertPrecompiledTouch_d0g0v0_Byzantium",
"RevertPrecompiledTouch_d0g0v0_Constantinople",
"RevertPrecompiledTouch_d0g0v0_ConstantinopleFix",
"RevertPrecompiledTouch_d0g0v0_EIP158",
"RevertPrecompiledTouch_d3g0v0_ConstantinopleFix",
"RevertPrecompiledTouchCC_d0g0v0_Byzantium",
"RevertPrecompiledTouchCC_d0g0v0_Constantinople",
"RevertPrecompiledTouchCC_d0g0v0_EIP158",
"RevertPrecompiledTouchDC_d0g0v0_Byzantium",
"RevertPrecompiledTouchDC_d0g0v0_Constantinople",
"RevertPrecompiledTouchDC_d0g0v0_EIP158",
"RevertPrecompiledTouchExactOOG_d7g1v0_ConstantinopleFix",
"RevertPrecompiledTouchExactOOG_d31g1v0_ConstantinopleFix",
"RevertPrecompiledTouch_storage_d3g0v0_ConstantinopleFix",
"RevertPrecompiledTouch_storage_d0g0v0_ConstantinopleFix"

Fix the tests and "un-skip" them.

NOTE: of the above all are fixed except two by applying @debris's fix from #10923 to ethcore/machine/src/executive.rs:

+                                       let ripemd = Address::from_low_u64_be(3);
+                                       if un_substate.touched.contains(&ripemd) {
+                                               substate.touched.insert(ripemd);
+                                       }

Failing blockchain tests with the above:

"RevertPrecompiledTouchExactOOG_d7g1v0_ConstantinopleFix",
"RevertPrecompiledTouchExactOOG_d31g1v0_ConstantinopleFix",
@dvdplm dvdplm added F4-tests 💻 Tests need fixing, improving or augmenting. P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible. labels Sep 20, 2019
@dvdplm dvdplm changed the title [json-tests] stRevertTest blockchain failues [json-tests] stRevertTest blockchain test failures Sep 20, 2019
@voith
Copy link

voith commented Sep 23, 2019

@dvdplm The same tests are failing in py-evm ethereum/py-evm#1852.
Did you get time to dig into these?
Since these tests are failing for two different clients, could this be an error in the upstream tests?

@adria0
Copy link

adria0 commented Apr 15, 2020

@dvdplm I think that I found what is happening here,

For instance, the test RevertPrecompiledTouchExactOOG_d7g1v0_ConstantinopleFix has the initial account 0000000000000000000000000000000000000008 (nonce=0,balance=0,code=0x) in the state trie. In the test, a call to this 0000....0008 address is done, with no changes in its state, and this account is cached. When State::kill_garbage() is executed, the function detects that the account exists_and_is_null() so kill_account(...) it.

Looking at the state trie root hash of go-ethereum, it does not kill this account, and it keeps alive. So OE differs how go-ethereum performs some check there.

Since this is in the precompile space, I tried to restrict account killing in precompiles >0xffff. Unfortunately, there is another test there failed_tx_xcf416c53_d0g0v0_ConstantinopleFix that seems that checks in the opposite direction: the account 0000...0003 (nonce=0,balance=0,code=0x) should be killed after the test. I did not dig a lot into the code but it seems that is calling contracts from 0x00 till 0x2b with zero gas.

I'm going to check how go-ethereum is exactly managing this, but maybe something comes to your mind with this info.

@ordian
Copy link
Collaborator

ordian commented Apr 15, 2020

the 0000...0003 is the rimemd address and it is treated differently for some legacy reasons, see the discussion and links in #10923

@adria0
Copy link

adria0 commented Apr 17, 2020

It seems that, when a "pre" allocation is done for accounts in the precompile space, geth (the filler of the tests) is calling the precompiles and OE is calling the code specified in the "pre" block. Asking for clarification.

ethereum/go-ethereum#20926

@adria0 adria0 linked a pull request Apr 28, 2020 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F4-tests 💻 Tests need fixing, improving or augmenting. P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants